SREP-1622: Fix endpoint admission test namespace creation for ROSA #30134
Conversation
|
/retitle OCPBUGS-60654: Fix endpoint admission test namespace creation for ROSA |
openshift-ci
bot
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@Amarthya-v: This pull request references Jira Issue OCPBUGS-60654, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
| _, err := adminClient.CoreV1().Namespaces().Get(context.Background(), namespace, metav1.GetOptions{}) | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| _, err = adminClient.CoreV1().Namespaces().Create(context.Background(), &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}, metav1.CreateOptions{}) |
There was a problem hiding this comment.
If the webhook does not allow creating a namespace, then I would expect this to also fail if the namespace does not exist by that time.
However I think kube-system namespace should always be there. Are you counting on that fact?
There was a problem hiding this comment.
@arghosh93 Yes, this fix specifically addresses the kube-system namespace case, which Always exists in OpenShift/Kubernetes clusters and is being unnecessarily created by the test, triggering the ROSA webhook
now it gets checked first before attempting creation.
For test-specific namespaces that don't exist, the create path still works as before. The webhook only blocks creation in managed namespaces like kube-system.
The failing ROSA jobs were specifically hitting this error when trying to create kube-system:
'''admission webhook "namespace-validation.managed.openshift.io" denied the request:
Prevented from accessing Red Hat managed namespaces
This fix avoids that unnecessary webhook call entirely for existing namespaces.'''
So the fix targets the specific scenario causing ROSA CI failures without breaking the general functionality.
|
/lgtm |
|
/assign kyrtapz |
|
@Amarthya-v why is the bug closed? |
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| } else { |
There was a problem hiding this comment.
NIT: there is no need for the else here.
To decrease the nestedness maybe we could go with:
if err != nil {
if !errors.IsNotFound(err) {
return nil, nil, err
}
_, err = adminClient.CoreV1().Namespaces().Create(context.Background(), &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}, metav1.CreateOptions{})
if err != nil {
return nil, nil, err
}
}
There was a problem hiding this comment.
@kyrtapz Updated! Much cleaner now. Thanks for the suggestion.
| @@ -210,4 +217,4 @@ func getClientForServiceAccount(adminClient kubernetes.Interface, clientConfig * | |||
| } | |||
|
|
|||
| return kubeClientset, saClientConfig, nil | |||
| } | |||
There was a problem hiding this comment.
Added the missing newline. thanks!
The bug was closed because Trevor reverted the webhook temporarily to unblock CI. This PR is the permanent fix - once merged, Trevor can safely re-enable the webhook without breaking ROSA CI. It's part of a coordinated plan: revert → fix test → re-enable webhook. |
Avoid attempting to create existing namespaces like kube-system which triggers managed-cluster-validating-webhooks in ROSA. Check if namespace exists before creation to prevent unnecessary webhook calls while maintaining same test behavior. Fixes: OCPBUGS-60654
Amarthya-v
force-pushed
the
fix-endpoint-admission-namespace-creation
branch
from
d09ba2e to
6389c5f
Compare
You need to address the |
Amarthya-v
changed the title
openshift-ci-robot
removed
the
jira/invalid-bug
|
@Amarthya-v: This pull request references SREP-1622 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 bug to target the "4.20.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. |
|
@Amarthya-v: This pull request references SREP-1622 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 bug to target the "4.20.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. |
1 similar comment
|
@Amarthya-v: This pull request references SREP-1622 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 bug to target the "4.20.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. |
feedback addressed and Jira issue resolved. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Amarthya-v, arghosh93, kyrtapz 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 |
openshift-ci
bot
added
the
approved
|
/override ci/prow/e2e-aws-ovn-fips |
|
@Amarthya-v: Amarthya-v unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight openshift-staff-engineers openshift-sustaining-engineers. 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 kubernetes-sigs/prow repository. |
|
/test e2e-aws-ovn-fips |
|
/skip |
1 similar comment
|
/skip |
|
/test e2e-aws-ovn-serial-1of2 |
|
/test e2e-gcp-ovn |
|
/skip |
|
/test e2e-vsphere-ovn |
|
@Amarthya-v: The following tests failed, say
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. |
Thanks Patryk!! the failure in ci/prow/e2e-vsphere-ovn My PR doesn’t modify networking, and I see the same failure in other jobs today. This looks like infra/kernel flake rather than my change. Requesting test override. |
Check if namespace exists before creation to prevent unnecessary webhook calls while maintaining same test behavior.
Fixes: SREP-1622