OCPBUGS-65634: add service account to verify pod#638
OCPBUGS-65634: add service account to verify pod#638ehearne-redhat wants to merge 3 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
jira/severity-critical
|
@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65634, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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-1
branch
from
d39797b to
7f4c112
Compare
|
/retest |
|
@oceanc80 after shift week, could you take a look? :) |
| err = k8sClient.Create(ctx, serviceAccount) | ||
| Expect(err).NotTo(HaveOccurred(), "failed to create Service Account") | ||
|
|
||
| if err != nil && !apierrors.IsAlreadyExists(err) { |
There was a problem hiding this comment.
We expect err to be nill on line 92. Maybe attenuate the Expect to be tolerant to AlreadyExists?
| serviceAccount := &corev1.ServiceAccount{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: jobNamePrefix, | ||
| Namespace: "default", |
There was a problem hiding this comment.
Wasn't the reversion due to the fact that we were still creating an SA in the default namespace?
There was a problem hiding this comment.
As far as I am aware, the reason why the revert occurred was due to race condition causing the curl job to not always consistently run and hence fail. The race condition being the service account was not cleaned up or applied before the curl job was applied.
There was some discussion in the Slack thread referenced in the revert #609 about having the verify pod in the default namespace as "suspicious". If you prefer I can move this pod to a different namespace. :)
|
/retest |
|
@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. |
5 participants
This PR addresses the revert that occurred in #609 . It includes test logic that ensures the job is deleted first before proceeding to delete the service account, assuming
DeferCleanupuses LIFO.