OCPBUGS-65634: UPSTREAM: <carry>: add service account to curl job#653
OCPBUGS-65634: UPSTREAM: <carry>: add service account to curl job#653ehearne-redhat wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
jira/severity-critical
|
@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65634, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ehearne-redhat The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ehearne-redhat
force-pushed
the
add-service-account-curl-job-2
branch
from
10893ab to
44e5264
Compare
|
@ehearne-redhat: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
ehearne-redhat
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
| // Create the Job | ||
| job := buildCurlJob(jobNamePrefix, "default", serviceURL, serviceAccount.Name) | ||
| err = k8sClient.Create(ctx, job) | ||
| Expect(err).NotTo(HaveOccurred(), "failed to create Job") |
There was a problem hiding this comment.
as it stands, you might have the service account hanging around if there is an issue creating the job. If we have something deleting the namespace, then that will likely clean up the service account. We may want to either add a comment to call this out, or if there is an error creating the job, delete the service account before raising the error
There was a problem hiding this comment.
I don't think it is possible to delete the default ns.
ehearne-mac:~ ehearne$ kubectl delete ns default
Error from server (Forbidden): namespaces "default" is forbidden: this namespace may not be deletedIf we were to move the job to a separate namespace just for running the jobs, and then when they are all done, delete the ns, then we can clean up completely.
Otherwise we could delete the service account when there is an error creating.
|
/payload-aggregate 4.22 10 |
|
@ehearne-redhat: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
|
/test ? |
|
@ehearne-redhat: The following commands are available to trigger required jobs: The following commands are available to trigger optional jobs: Use DetailsIn 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 kubernetes-sigs/prow repository. |
|
/payload-aggregate openshift-e2e-aws 10 |
|
@ehearne-redhat: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
|
/payload-aggregate pull-ci-openshift-operator-framework-operator-controller-main-openshift-e2e-aws 10 |
|
@ehearne-redhat: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
|
/payload-aggregate ? |
|
@ehearne-redhat: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info. |
|
/payload-aggregate periodic-ci-openshift-release-main-ci-4.22-upgrade-from-stable-4.21-e2e-aws-ovn-upgrade 10 |
|
@ehearne-redhat: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/aa6f9100-16eb-11f1-8196-e62b76003c3a-0 |
4 participants
This PR addresses the revert that occurred in #638 .
We believe the changes should ensure the
Pendingstate of verify pods does not happen, because:DeferCleanup()block being used to handle job and service account cleanup.deletePolicy := metav1.DeletePropagationForegroundensuring all dependent pods (which there should be only one) are deleted andgracePeriod := int64(0)ensuring an instant deletion. The job can be forcefully deleted, becauseDeferCleanup()would not occur unless the job successfully ran.Eventually()block between the job deletion and service account deletion will prevent service account deletion from occurring if there was an issue with deleting the job. This means that any pods still running associated with that job should not get stuck in aPendingstate for service account related reasons.We are hopeful that the following changes should not cause a revert, and it should be using proper commit message formatting.