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

Wait for the UDN namespaces to have SA's created #29522

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tssurya
Copy link
Contributor

@tssurya tssurya commented Feb 3, 2025

Should fix: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn-techpreview/1885825937673555968

pods "red-pod-0" is forbidden: error looking up service account e2e-network-segmentation-e2e-6878-red/default: serviceaccount "default" not found
    {
        ErrStatus:
            code: 403
            details:
              kind: pods
              name: red-pod-0
            message: 'pods "red-pod-0" is forbidden: error looking up service account e2e-network-segmentation-e2e-6878-red/default:
              serviceaccount "default" not found'
            metadata: {}
            reason: Forbidden
            status: Failure,

@trozet already fixed something similar for SNO in 20a6022 from what I can tell we missed a few spots for the custom namespaces we create ourselves outside the test framework...

@openshift-ci openshift-ci bot requested review from knobunc and trozet February 3, 2025 12:10
@tssurya
Copy link
Contributor Author

tssurya commented Feb 3, 2025

/payload-aggregate periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn-techpreview 10

Copy link
Contributor

openshift-ci bot commented Feb 3, 2025

@tssurya: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/2e0ae590-e228-11ef-905e-e1dff8a8dba4-0

Copy link
Contributor

@maiqueb maiqueb left a comment

Choose a reason for hiding this comment

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

/lgtm

My only "beef" with this if that the expectation could be inlined.
Still, that's peanuts.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2025
Copy link
Contributor

openshift-ci bot commented Feb 3, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: maiqueb, tssurya
Once this PR has been reviewed and has the lgtm label, please assign trozet for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tssurya
Copy link
Contributor Author

tssurya commented Feb 3, 2025

/test e2e-gcp-ovn-techpreview

@tssurya
Copy link
Contributor Author

tssurya commented Feb 3, 2025

/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-ipv6-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-azure-ovn-runc-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-dualstack-techpreview

Copy link
Contributor

openshift-ci bot commented Feb 3, 2025

@tssurya: trigger 4 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn-techpreview
  • periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-ipv6-techpreview
  • periodic-ci-openshift-release-master-nightly-4.19-e2e-azure-ovn-runc-techpreview
  • periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-dualstack-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/2acb7c90-e229-11ef-824f-b8ee96cdc719-0

Copy link
Contributor

@trozet trozet left a comment

Choose a reason for hiding this comment

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

I think you should change the namespace creation in these spots to use CreateNamespace

@tssurya
Copy link
Contributor Author

tssurya commented Feb 3, 2025

I think you should change the namespace creation in these spots to use CreateNamespace

ack yea I wasn't sure if that was not done intentionally or not... let me update the PR to use f.CreateNamespace

@tssurya
Copy link
Contributor Author

tssurya commented Feb 3, 2025

I think you should change the namespace creation in these spots to use CreateNamespace

ack yea I wasn't sure if that was not done intentionally or not... let me update the PR to use f.CreateNamespace

I have fixed this now gonna repush given https://prow.ci.openshift.org/view/gs/test-platform-results/logs/openshift-origin-29522-nightly-4.19-e2e-vsphere-ovn-techpreview/1886389139251662848 passed with 0 UDN flakes

@tssurya tssurya force-pushed the wait-for-SA-creation-in-UDN-namespaces branch from a3f4d4d to c07df86 Compare February 3, 2025 14:37
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2025
Copy link
Contributor

openshift-ci bot commented Feb 3, 2025

New changes are detected. LGTM label has been removed.

Should fix:

pods "red-pod-0" is forbidden: error looking up service account e2e-network-segmentation-e2e-6878-red/default: serviceaccount "default" not found
    {
        ErrStatus:
            code: 403
            details:
              kind: pods
              name: red-pod-0
            message: 'pods "red-pod-0" is forbidden: error looking up service account e2e-network-segmentation-e2e-6878-red/default:
              serviceaccount "default" not found'
            metadata: {}
            reason: Forbidden
            status: Failure,

Also change the vanilla create namespaces calls
done in the tests to use the framework create

Signed-off-by: Surya Seetharaman <[email protected]>
@tssurya tssurya force-pushed the wait-for-SA-creation-in-UDN-namespaces branch from c07df86 to 1af2b25 Compare February 3, 2025 14:41
@tssurya
Copy link
Contributor Author

tssurya commented Feb 3, 2025

/test e2e-gcp-ovn-techpreview

@tssurya
Copy link
Contributor Author

tssurya commented Feb 3, 2025

/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-ipv6-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-azure-ovn-runc-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-dualstack-techpreview

Copy link
Contributor

openshift-ci bot commented Feb 3, 2025

@tssurya: trigger 4 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn-techpreview
  • periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-ipv6-techpreview
  • periodic-ci-openshift-release-master-nightly-4.19-e2e-azure-ovn-runc-techpreview
  • periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-dualstack-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4ce76410-e23d-11ef-8090-74b7cfcc80b1-0

Name: defaultNetNamespace,
},
}, metav1.CreateOptions{})
_, err = f.CreateNamespace(context.TODO(), defaultNetNamespace, nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

question @tssurya: don't we need to err = udnWaitForOpenShift(oc, namespace) here ?

Copy link
Contributor Author

@tssurya tssurya Feb 4, 2025

Choose a reason for hiding this comment

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

because that namespaces isn't a UDN namespace... unsure if that udnWaitForOCP is doing generic things or not.
but on a high level - it does make to add this wait also for the non UDN namespaces huh? i had thought of adding it for the default namespaces as well.. maybe we should like you are saying

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm unsure ...

@tssurya
Copy link
Contributor Author

tssurya commented Feb 4, 2025

/hold
CI is failing

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 4, 2025
Copy link
Contributor

openshift-ci bot commented Feb 12, 2025

@tssurya: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-single-node 1af2b25 link false /test e2e-aws-ovn-single-node
ci/prow/e2e-aws-ovn-single-node-upgrade 1af2b25 link false /test e2e-aws-ovn-single-node-upgrade
ci/prow/e2e-gcp-ovn 1af2b25 link true /test e2e-gcp-ovn
ci/prow/e2e-gcp-csi 1af2b25 link false /test e2e-gcp-csi
ci/prow/e2e-aws-ovn-single-node-serial 1af2b25 link false /test e2e-aws-ovn-single-node-serial
ci/prow/e2e-openstack-ovn 1af2b25 link false /test e2e-openstack-ovn
ci/prow/e2e-gcp-ovn-techpreview 1af2b25 link false /test e2e-gcp-ovn-techpreview
ci/prow/e2e-vsphere-ovn 1af2b25 link true /test e2e-vsphere-ovn
ci/prow/e2e-vsphere-ovn-upi 1af2b25 link true /test e2e-vsphere-ovn-upi

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.

Copy link

openshift-trt bot commented Feb 12, 2025

Job Failure Risk Analysis for sha: 1af2b25

Job Name Failure Risk
pull-ci-openshift-origin-master-e2e-gcp-ovn-techpreview IncompleteTests
Tests for this run (105) are below the historical average (2667): IncompleteTests (not enough tests ran to make a reasonable risk analysis; this could be due to infra, installation, or upgrade problems)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants