Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wait option to Pod Create #44

Merged
merged 5 commits into from
May 16, 2022

Conversation

pablochacin
Copy link
Contributor

@pablochacin pablochacin commented May 9, 2022

Add option for waiting until the Pod is running to pod Create. if the pod is not running after a given timeout, an error is returned.

Closes #43

Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
@pablochacin pablochacin changed the title Add wait option to Pod Create WIP: Add wait option to Pod Create May 9, 2022
@ppcano
Copy link
Contributor

ppcano commented May 10, 2022

Given the example:

kubernetes.pods.create({
    namespace: namespace,
    name: podName,
    image: image,
    command: command
  })

const options = {
    namespace: namespace,
    name: podName,
    status: "Succeeded",
    timeout: "10s"
}
if (kubernetes.pods.wait(options)) {
    console.log(podName + " pod completed successfully")
} else {
    throw podName + " is not completed"
}

I suggest adding the timeout option to the creation command. It could be something like:

const resultExecution = kubernetes.pods.createAndWait({
    namespace: namespace,
    name: podName,
    image: image,
    command: command,
    timeout: "10s"
})

or any other alternative:

  • kubernetes.pods.sendCommand: and timeout is required.
  • ....

@pablochacin
Copy link
Contributor Author

I suggest adding the timeout option to the creation command. It could be something like:

@ppcano this is already done. I just also made possible to wait after the pod is created. This is useful for waiting for the pod to complete.

@ppcano ppcano requested a review from javaducky May 11, 2022 13:22
Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some unit tests around this new functionality; basically what the example scripts attempt to prove.

Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
@pablochacin pablochacin changed the title WIP: Add wait option to Pod Create Add wait option to Pod Create May 14, 2022
Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple minor issues.

pkg/pods/pods.go Show resolved Hide resolved
pkg/pods/pods_test.go Outdated Show resolved Hide resolved
pkg/pods/pods_test.go Outdated Show resolved Hide resolved
pkg/pods/pods.go Outdated Show resolved Hide resolved
const namespace = "default"
const podName = "new-pod"
const image = "busybox"
const command = ["sh", "-c", "/bin/false"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this command puts the pod into an error state resulting in script returning Pod has failed. Was this the intended behavior for the example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the example to (hopefully) make the purpose evident.

Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you for a great addition!

@javaducky javaducky merged commit 1adb016 into grafana:main May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pods.create: check the successful creation
3 participants