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

tests/e2e: General fixes to the operator script #338

Merged

Conversation

GabyCT
Copy link
Contributor

@GabyCT GabyCT commented Jan 23, 2024

This PR introduces general fixes to the test/e2e operator script, like fixing the indentation, using a variable that has been already declared and use in the script.

@@ -266,7 +265,7 @@ wait_for_stabilization() {
count=0
while true; do
change=0
pod_info=$(kubectl get pods -n confidential-containers-system -o=jsonpath='{range .items[*]}{.metadata.name}{" "}{range .status.containerStatuses[*]}{.name}{" "}{.restartCount}{"\n"}{end}{end}')
pod_info=$(kubectl get pods -n $op_ns -o=jsonpath='{range .items[*]}{.metadata.name}{" "}{range .status.containerStatuses[*]}{.name}{" "}{.restartCount}{"\n"}{end}{end}')
Copy link
Contributor

Choose a reason for hiding this comment

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

To be sure it'd be better to quotate the $op_ns (just in case a space/whatever-weird-char is ever added)

Copy link
Contributor

@ldoktor ldoktor left a comment

Choose a reason for hiding this comment

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

Thanks, unifying is always good (although I have to say I'm a 4-space person :-/). Please consider the quotation, even though it should not be needed it's a good practice to do so.

@GabyCT
Copy link
Contributor Author

GabyCT commented Jan 24, 2024

Thanks, unifying is always good (although I have to say I'm a 4-space person :-/). Please consider the quotation, even though it should not be needed it's a good practice to do so.

@ldoktor thanks for the feedback and changes applied :)

@portersrc
Copy link
Member

lgtm!

Copy link
Contributor

@ldoktor ldoktor left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm

Copy link
Member

@wainersm wainersm left a comment

Choose a reason for hiding this comment

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

Thanks @GabyCT !

@wainersm
Copy link
Member

Let's see if #342 fixes the e2e-pr failures

@wainersm
Copy link
Member

Let's see if #342 fixes the e2e-pr failures

PR #342 is merged now. @GabyCT could you please rebase this PR so we can re-run the CI?

This PR introduces general fixes to the test/e2e operator script, like
fixing the indentation, using a variable that has been already declared
and use in the script.

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
@GabyCT
Copy link
Contributor Author

GabyCT commented Jan 30, 2024

Let's see if #342 fixes the e2e-pr failures

PR #342 is merged now. @GabyCT could you please rebase this PR so we can re-run the CI?

@wainersm I just did the rebase :)

@wainersm wainersm merged commit ee79ae5 into confidential-containers:main Jan 31, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants