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

[WIP] Stop using dev-env in E2E setup #2250

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

[WIP] Stop using dev-env in E2E setup #2250

wants to merge 1 commit into from

Conversation

mquhuy
Copy link
Member

@mquhuy mquhuy commented Jan 16, 2025 •
edited
Loading

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1996

TODO

  • Using 2 redfish nodes and 2 ipmi?
  • Incorporate fake-ipa setting from dev-env

@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 Jan 16, 2025
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign adilghaffardev 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

@metal3-io-bot metal3-io-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 16, 2025
@mquhuy mquhuy force-pushed the mquhuy/detach-dev-env-from-e2e branch 2 times, most recently from 31517f2 to 07202f6 Compare January 16, 2025 15:46
@mquhuy
Copy link
Member Author

mquhuy commented Jan 16, 2025

/test ?

@metal3-io-bot
Copy link
Contributor

@mquhuy: The following commands are available to trigger required jobs:

/test build

/test generate

/test gomod

/test manifestlint

/test markdownlint

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

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

/test shellcheck

/test test

/test unit

The following commands are available to trigger optional jobs:

/test metal3-centos-e2e-basic-test-main

/test metal3-centos-e2e-feature-test-main-features

/test metal3-centos-e2e-feature-test-main-pivoting

/test metal3-centos-e2e-feature-test-main-remediation

/test metal3-e2e-1-29-1-30-upgrade-test-main

/test metal3-e2e-clusterctl-upgrade-test-main

/test metal3-ubuntu-e2e-basic-test-main

/test metal3-ubuntu-e2e-feature-test-main-features

/test metal3-ubuntu-e2e-feature-test-main-pivoting

/test metal3-ubuntu-e2e-feature-test-main-remediation

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

build

generate

gomod

manifestlint

shellcheck

test

unit

In response to this:

/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.

@mquhuy
Copy link
Member Author

mquhuy commented Jan 16, 2025

/test metal3-ubuntu-e2e-feature-test-main-pivoting

@mquhuy mquhuy force-pushed the mquhuy/detach-dev-env-from-e2e branch from 07202f6 to beb15b0 Compare January 16, 2025 15:56
@mquhuy
Copy link
Member Author

mquhuy commented Jan 16, 2025

/test metal3-ubuntu-e2e-feature-test-main-pivoting

@mquhuy mquhuy force-pushed the mquhuy/detach-dev-env-from-e2e branch from beb15b0 to a78401b Compare January 16, 2025 16:00
@mquhuy
Copy link
Member Author

mquhuy commented Jan 16, 2025

/test metal3-ubuntu-e2e-feature-test-main-pivoting

tuminoid
tuminoid reviewed Jan 17, 2025
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 early nitting.

edit: some of it is already catched by the linters too

hack/e2e/fake-ipa.sh
@@ -0,0 +1,34 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#!/bin/bash
#!/usr/bin/env bash

scripts/ci-e2e.sh
@@ -3,21 +3,36 @@
set -euxo pipefail
Copy link
Member

Choose a reason for hiding this comment

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

Fix the bin/bash on L1 here too to usr/bin/env bash

scripts/ci-e2e.sh

"${REPO_ROOT}/hack/ensure-go.sh"
# shellcheck source=./hack/ensure-kind.sh
source "${REPO_ROOT}/hack/ensure-kind.sh"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
source "${REPO_ROOT}/hack/ensure-kind.sh"
. "${REPO_ROOT}/hack/ensure-kind.sh"

Same for below lines as well.

scripts/ci-e2e.sh Outdated
echo 'export EPHEMERAL_CLUSTER="minikube"' >>"${M3_DEV_ENV_PATH}/config_${USER}.sh"
BMC_PROTOCOL="${BMC_PROTOCOL:-"redfish-virtualmedia"}"
if [[ "${BMC_PROTOCOL}" == "redfish" ]] || [[ "${BMC_PROTOCOL}" == "redfish-virtualmedia" ]]; then
BMO_E2E_EMULATOR="sushy-tools"
Copy link
Member

Choose a reason for hiding this comment

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

Keep the indents at 4, consistency.

scripts/ci-e2e.sh Show resolved Hide resolved
test/e2e/data/ironic-deployment/overlays/release-latest/ironic-patch.yaml
template:
spec:
containers:
- name: ironic-dnsmasq
Copy link
Member

Choose a reason for hiding this comment

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

indent

test/e2e/e2e_suite_test.go
@@ -76,10 +85,12 @@ var (
numberOfControlplane int
numberOfWorkers int
numberOfAllBmh int
err error
Copy link
Member

Choose a reason for hiding this comment

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

are these not catched by linters?

Copy link
Member

Choose a reason for hiding this comment

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

Oh they are.

@mquhuy mquhuy force-pushed the mquhuy/detach-dev-env-from-e2e branch 2 times, most recently from 407e44e to d67a19f Compare January 17, 2025 07:58
@metal3-io-bot metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Jan 17, 2025
@metal3-io-bot
Copy link
Contributor

PR needs rebase.

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.

@tuminoid
Copy link
Member

This is WIP, but anyways
/hold
as CI is a mess, so let's not merge this before we have tests green again.

@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 Jan 17, 2025
@mquhuy mquhuy force-pushed the mquhuy/detach-dev-env-from-e2e branch from d67a19f to 6b4cff0 Compare January 17, 2025 13:04
@mquhuy mquhuy force-pushed the mquhuy/detach-dev-env-from-e2e branch from 6b4cff0 to 3d102b0 Compare January 17, 2025 13:31
@metal3-io-bot
Copy link
Contributor

@mquhuy: 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
metal3-ubuntu-e2e-feature-test-main-pivoting a78401b link false /test metal3-ubuntu-e2e-feature-test-main-pivoting
gomod 3d102b0 link true /test gomod
manifestlint 3d102b0 link true /test manifestlint
shellcheck 3d102b0 link true /test shellcheck
unit 3d102b0 link true /test unit
generate 3d102b0 link true /test generate
build 3d102b0 link true /test build
test 3d102b0 link true /test test

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.

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

@tuminoid tuminoid tuminoid left review comments

@smoshiur1237 smoshiur1237 Awaiting requested review from smoshiur1237

Assignees
No one assigned
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Decouple CAPM3 from metal3-dev-env
3 participants
@mquhuy @metal3-io-bot @tuminoid