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.

Sign up for GitHub

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

Jump to bottom

Add test against cda cluster to validate sfc behavior #201

Merged
merged 2 commits into from
Jan 17, 2025
Merged

Add test against cda cluster to validate sfc behavior #201

merged 2 commits into from
Jan 17, 2025

Conversation

bn222
Copy link
Contributor

@bn222 bn222 commented Oct 31, 2024

No description provided.

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 31, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bn222

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 31, 2024
@bn222
Copy link
Contributor Author

bn222 commented Oct 31, 2024

/test make-e2e-test

@SalDaniele
Copy link
Contributor

Nice!

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2024
@SalDaniele
Copy link
Contributor

/retest ci/prow/make-fast-e2e-test

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 31, 2024

@SalDaniele: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test images
  • /test make-e2e-test
  • /test make-fast-e2e-test
  • /test make-fmt-check
  • /test make-generate-check
  • /test make-prow-ci-manifests-check
  • /test make-test
  • /test make-vendor-check

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-dpu-operator-main-images
  • pull-ci-openshift-dpu-operator-main-make-fast-e2e-test
  • pull-ci-openshift-dpu-operator-main-make-fmt-check
  • pull-ci-openshift-dpu-operator-main-make-generate-check
  • pull-ci-openshift-dpu-operator-main-make-prow-ci-manifests-check
  • pull-ci-openshift-dpu-operator-main-make-test
  • pull-ci-openshift-dpu-operator-main-make-vendor-check

In response to this:

/retest ci/prow/make-fast-e2e-test

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.

@SalDaniele
Copy link
Contributor

/test ci/prow/make-fast-e2e-test

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 31, 2024

@SalDaniele: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test images
  • /test make-e2e-test
  • /test make-fast-e2e-test
  • /test make-fmt-check
  • /test make-generate-check
  • /test make-prow-ci-manifests-check
  • /test make-test
  • /test make-vendor-check

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-dpu-operator-main-images
  • pull-ci-openshift-dpu-operator-main-make-fast-e2e-test
  • pull-ci-openshift-dpu-operator-main-make-fmt-check
  • pull-ci-openshift-dpu-operator-main-make-generate-check
  • pull-ci-openshift-dpu-operator-main-make-prow-ci-manifests-check
  • pull-ci-openshift-dpu-operator-main-make-test
  • pull-ci-openshift-dpu-operator-main-make-vendor-check

In response to this:

/test ci/prow/make-fast-e2e-test

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.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 4598126 and 2 for PR HEAD c7a6da9 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 3dfc39d and 1 for PR HEAD c7a6da9 in total

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 1, 2024
thom311
thom311 reviewed Nov 4, 2024
View reviewed changes
e2e-test/e2e_test.go Outdated Show resolved Hide resolved
@thom311
Copy link
Contributor

thom311 commented Nov 4, 2024 •
edited
Loading

Could you add a README (or a comment somewhere) how such a test can run?

Obviously, it depends on the ipu_deploy target. Which turns the request into how to run the ipu_deploy target.

Since the CDA config in hack/cluster-configs/config-dpu.yaml reads the google-spread sheet, it seems clear that it can only work on machines from that spread sheet. If that is not the case, how to run the target on another system? If that is the case, which machines exactly? (also considering that some of those machines are hooked up to CI, it's not clear which can be taken for manual testing -- if any).

Point being: please explain this somewhere (a README would be good).

Also, it does seem to me that all those make e2e-test targets really only work on certain machines and are not usable to an upstream user (unless they patch parts of the code). With that, it seems strange that these are make targets (that look usable to anybody, when they are not). I think these should just be scripts under contrib/rh-ci or similar. Or if not and I just don't understand it, how can a user who is not in our CI lab use those targets? The point here is, why is this all hooked up as make targets, when it could just be scripts (in a clearly named directory). Well, the make targets are already just calling to scripts, so why do they exist at all?

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2024
@bn222
Copy link
Contributor Author

bn222 commented Nov 5, 2024

/override ci/prow/make-fast-e2e-test

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 5, 2024

@bn222: Overrode contexts on behalf of bn222: ci/prow/make-fast-e2e-test

In response to this:

/override ci/prow/make-fast-e2e-test

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.

@openshift-ci-robot
Copy link

/test remaining-required

@bn222
Copy link
Contributor Author

bn222 commented Nov 20, 2024

/test make-e2e-test

@SalDaniele
Copy link
Contributor

/lgtm

@SalDaniele
Copy link
Contributor

/test make-e2e-test

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2024
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 21, 2024
@SalDaniele SalDaniele mentioned this pull request Nov 26, 2024
Closed
@openshift-ci-robot
Copy link

/test remaining-required

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2024
@SalDaniele
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2024
@openshift-ci-robot
Copy link

/test remaining-required

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 99f3e30 and 2 for PR HEAD 1725d51 in total

3 similar comments
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 99f3e30 and 2 for PR HEAD 1725d51 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 99f3e30 and 2 for PR HEAD 1725d51 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 99f3e30 and 2 for PR HEAD 1725d51 in total

thom311
thom311 reviewed Dec 3, 2024
View reviewed changes
internal/testutils/cdacluster.go Show resolved Hide resolved
e2e-test/e2e_test.go Outdated Show resolved Hide resolved
internal/testutils/cdacluster.go Outdated Show resolved Hide resolved
e2e-test/e2e_test.go Outdated Show resolved Hide resolved
e2e-test/e2e_test.go Outdated Show resolved Hide resolved
e2e-test/e2e_test.go Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 7, 2025
@bn222 bn222 force-pushed the e2e-scaffold branch 2 times, most recently from 8aa82c0 to b21eecd Compare January 8, 2025 15:54
@SalDaniele
Copy link
Contributor

/lgtm

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 17, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jan 17, 2025

@bn222: 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.

@openshift-merge-bot openshift-merge-bot bot merged commit 0aa87a5 into openshift:main Jan 17, 2025
8 checks passed
@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: dpu-cni
This PR has been included in build dpu-cni-container-v4.19.0-202501172138.p0.g0aa87a5.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: dpu-operator
This PR has been included in build dpu-operator-container-v4.19.0-202501172138.p0.g0aa87a5.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: dpu-daemon
This PR has been included in build dpu-daemon-container-v4.19.0-202501172138.p0.g0aa87a5.assembly.stream.el9.
All builds following this will include this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thom311 thom311 thom311 left review comments

@SalDaniele SalDaniele Awaiting requested review from SalDaniele

@wizhaoredhat wizhaoredhat Awaiting requested review from wizhaoredhat

Assignees

@SalDaniele SalDaniele

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@bn222 @SalDaniele @openshift-ci-robot @thom311 @vrindle @openshift-bot @openshift-merge-robot