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

🌱 Integrate sushy-tools and Redfish into e2e testing #1410

Merged
merged 1 commit into from
Nov 6, 2023
Merged

🌱 Integrate sushy-tools and Redfish into e2e testing #1410

merged 1 commit into from
Nov 6, 2023

Conversation

maxrantil
Copy link
Member

What this PR does / why we need it:
This PR augments the e2e testing framework by integrating sushy-tools and Redfish, enhancing test coverage across different management interfaces alongside the existing VBMC and IPMI tests.

Fixes #1374

@metal3-io-bot metal3-io-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 30, 2023
@maxrantil
Copy link
Member Author

/hold

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 30, 2023
@kashifest kashifest changed the title 🌱 Integrate sushy-tools and Redfish into e2e testing WIP: 🌱 Integrate sushy-tools and Redfish into e2e testing Nov 1, 2023
@metal3-io-bot metal3-io-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 1, 2023
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 1, 2023
@maxrantil
Copy link
Member Author

/hold cancel

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 2, 2023
@maxrantil maxrantil changed the title WIP: 🌱 Integrate sushy-tools and Redfish into e2e testing 🌱 Integrate sushy-tools and Redfish into e2e testing Nov 2, 2023
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 2, 2023
@maxrantil
Copy link
Member Author

/metal3-bmo-e2e-test

tuminoid
tuminoid reviewed Nov 2, 2023
View reviewed changes
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

Some comments and nits.

hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
@maxrantil maxrantil force-pushed the test-redfish-e2e/max branch 3 times, most recently from 5b0a43d to 9346ed5 Compare November 2, 2023 07:51
@maxrantil
Copy link
Member Author

/metal3-bmo-e2e-test
/test-ubuntu-integration-main
/test-centos-e2e-integration-main

@maxrantil
Copy link
Member Author

/metal3-bmo-e2e-test
/test-ubuntu-integration-main
/test-centos-e2e-integration-main

@maxrantil
Copy link
Member Author

/cc @lentzi90 @kashifest @dtantsur

dtantsur
dtantsur reviewed Nov 2, 2023
View reviewed changes
hack/ci-e2e.sh Outdated Show resolved Hide resolved
dtantsur
dtantsur reviewed Nov 2, 2023
View reviewed changes
test/e2e/sushy-tools/sushy-emulator.conf Outdated
@@ -0,0 +1,36 @@
# Listen on all local IP interfaces
SUSHY_EMULATOR_LISTEN_IP = u'0.0.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

This is not great. You use neither authentication nor TLS. This will open VM management on your machine to anyone. Cannot we assume localhost? If not, I'd probably add TLS and auth.

Copy link
Member

Choose a reason for hiding this comment

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

Good point! I think we can use 192.168.222.1 here. Localhost will probably not work since Ironic would not be able to reach it then.

lentzi90
lentzi90 reviewed Nov 2, 2023
View reviewed changes
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

Great job once again!
Some comments below 🙂

test/e2e/config/ironic.yaml Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2023
@lentzi90
Copy link
Member

lentzi90 commented Nov 3, 2023

Issues with the jenkins workers...
/metal3-bmo-e2e-test

mquhuy
mquhuy reviewed Nov 3, 2023
View reviewed changes
hack/clean-e2e.sh Outdated Show resolved Hide resolved
mquhuy
mquhuy reviewed Nov 3, 2023
View reviewed changes
Copy link
Member

@mquhuy mquhuy left a comment

Choose a reason for hiding this comment

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

It looks very good! I just have some minor comments.

@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2023
lentzi90
lentzi90 reviewed Nov 3, 2023
View reviewed changes
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2023
@mquhuy
Copy link
Member

mquhuy commented Nov 3, 2023

/test-centos-e2e-integration-main
/test-ubuntu-integration-main

@lentzi90
Copy link
Member

lentzi90 commented Nov 3, 2023

/metal3-bmo-e2e-test

1 similar comment
@lentzi90
Copy link
Member

lentzi90 commented Nov 3, 2023

/metal3-bmo-e2e-test

@maxrantil
Copy link
Member Author

/test-centos-e2e-integration-main
/test-ubuntu-integration-main

Integrate sushy-tools and Redfish into e2e testing
728b6cf 
Extended the e2e testing framework to include sushy-tools and Redfish
alongside existing VBMC and IPMI tests.
@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2023
@maxrantil
Copy link
Member Author

/metal3-bmo-e2e-test

@maxrantil
Copy link
Member Author

/test-centos-e2e-integration-main
/test-ubuntu-integration-main

@elfosardo
Copy link
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2023
@lentzi90
Copy link
Member

lentzi90 commented Nov 3, 2023

/override test-centos-e2e-integration-main
/override test-ubuntu-integration-main

@metal3-io-bot
Copy link
Contributor

@lentzi90: lentzi90 unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file.

In response to this:

/override test-centos-e2e-integration-main
/override test-ubuntu-integration-main

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/test-infra repository.

@maxrantil
Copy link
Member Author

/test-centos-e2e-integration-main
/test-ubuntu-integration-main

kashifest
kashifest reviewed Nov 6, 2023
View reviewed changes
Copy link
Member

@kashifest kashifest left a comment

Choose a reason for hiding this comment

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

/approve

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest

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

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 6, 2023
@kashifest
Copy link
Member

/override test-centos-e2e-integration-main
/override test-ubuntu-integration-main

@metal3-io-bot
Copy link
Contributor

@kashifest: Overrode contexts on behalf of kashifest: test-centos-e2e-integration-main, test-ubuntu-integration-main

In response to this:

/override test-centos-e2e-integration-main
/override test-ubuntu-integration-main

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/test-infra repository.

@metal3-io-bot metal3-io-bot merged commit 7e85b3d into metal3-io:main Nov 6, 2023
14 of 15 checks passed
@metal3-io-bot metal3-io-bot deleted the test-redfish-e2e/max branch November 6, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtantsur dtantsur dtantsur left review comments

@tuminoid tuminoid tuminoid left review comments

@lentzi90 lentzi90 lentzi90 left review comments

@mquhuy mquhuy mquhuy left review comments

@kashifest kashifest kashifest left review comments

Assignees
No one assigned
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test Redfish in e2e tests
8 participants
@maxrantil @lentzi90 @mquhuy @elfosardo @metal3-io-bot @kashifest @dtantsur @tuminoid