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

No err message on node found #164

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mimiteto
Copy link
Contributor

What this PR does / why we need it:
PR fixes the case where a node is found but err message is still printed in hacks/ops-pod.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:


@mimiteto mimiteto requested a review from a team as a code owner December 13, 2024 12:14
@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Dec 13, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Dec 13, 2024
hacks/ops-pod Outdated
@@ -111,7 +111,7 @@ if [[ -n ${node_of_pod} ]]; then
else
echo "Node name provided ..."
# shellcheck disable=SC2076
Copy link
Contributor

Choose a reason for hiding this comment

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

This should no-longer be necessary: https://www.shellcheck.net/wiki/SC2076

@@ -111,7 +111,7 @@ if [[ -n ${node_of_pod} ]]; then
else
echo "Node name provided ..."
# shellcheck disable=SC2076
if [[ $(kubectl get nodes -o jsonpath='{.items[*].metadata.name}') =~ " ${node} " ]]; then
if kubectl get nodes -o jsonpath='{.items[*].metadata.name}' | grep -q "\b${node}\b"; then
echo -e "Deploying ops pod on ${node}\n"
else
echo -e "Error: node ${node} does not exist in the cluster.\n"
Copy link
Contributor

@plkokanov plkokanov Dec 13, 2024

Choose a reason for hiding this comment

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

Can we use this PR to also add an exit 1 after the print_usage call so that we do not try to start up a pod that will never become Ready since the node does not exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actaully, should print_usage also exit?

@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Dec 13, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 13, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants