-
Notifications
You must be signed in to change notification settings - Fork 196
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
USHIFT-3742: test if all the running pods are annotated for workload partitioning #3608
USHIFT-3742: test if all the running pods are annotated for workload partitioning #3608
Conversation
87ce9e4
to
0ac6042
Compare
@eslutsky: This pull request references USHIFT-3742 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
2afa28c
to
8f0d521
Compare
@{pods}= Split String ${pods_raw} | ||
FOR ${pod} IN @{pods} | ||
${ns} ${pod}= Split String ${pod} \@ | ||
${csv_dss}= Check Pod Management Annotation ${ns} ${pod} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not look like the check function returns any values, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no it doesnt return, changed the name.
Oc Get All Pods | ||
[Documentation] Runs 'oc get pods -A` | ||
... Returns the command output as formatted string <name-space>@<pod-name> | ||
|
||
${data}= Run With Kubeconfig | ||
... oc get pods -A -o jsonpath='{range .items[*]}{.metadata.namespace}{"@"}{.metadata.name}{"\\n"}{end}' | ||
RETURN ${data} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reuse the existing Oc Get JsonPath
function from oc.resource
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could, but the original function Oc Get JsonPath
only works with a specified namespace, the idea here is that we want to get all the pods across all namespaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we would need to refactor it to support -A
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think let's refactor rather than creating a copy of the code.
The easiest way should be to check the namespace argument value and if it's empty, pass -A to the command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
8f0d521
to
3a76047
Compare
/retest |
1 similar comment
/retest |
Signed-off-by: Evgeny Slutsky <eslutsky@redhat.com>
3a76047
to
ded674c
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eslutsky, ggiguash The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@eslutsky: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Which issue(s) this PR addresses:
Closes #