Make minikube the default ephemeral cluster #1404

kashifest · 2024-05-20T06:01:15Z

This PR makes minikube the default ephemeral cluster irrespective of container runtime or image OS. The idea is to run Ubuntu tests only for the time being in different repos until CI is stable. Centos e2es are failing more often. Making minikube ephemeral cluster makes sure that we test ironic as a k8s deployment instead of local containers.

kashifest · 2024-05-20T06:01:32Z

/hold
lets test it properly first.

kashifest · 2024-05-20T06:01:51Z

/test metal3-dev-env-integration-test-ubuntu-main

This PR makes minikube the default ephemeral cluster irrespective of container runtime or image OS. The idea is to run Ubuntu tests only for the time being in different repos until CI is stable. Centos e2es are failing more often. Making minikube ephemeral cluster makes sure that we test ironic as a k8s deployment instead of local containers. Signed-off-by: Kashif Khan <kashif.khan@est.tech>

kashifest · 2024-05-20T12:27:34Z

/test metal3-dev-env-integration-test-ubuntu-main

mboukhalfa · 2024-05-20T16:55:51Z

Can you create an issue to keep track and add TODOs once centos back to normal

kashifest · 2024-05-21T06:19:02Z

Can you create an issue to keep track and add TODOs once centos back to normal

#1405

kashifest · 2024-05-21T06:20:46Z

/test metal3-dev-env-integration-test-ubuntu-release-1.7
/test metal3-dev-env-integration-test-ubuntu-release-1.6

metal3-io-bot · 2024-05-21T06:20:49Z

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

/test markdownlint
/test metal3-centos-e2e-integration-test-release-1-7
/test metal3-dev-env-integration-test-ubuntu-main
/test shellcheck

The following commands are available to trigger optional jobs:

/test metal3-centos-e2e-basic-test-main
/test metal3-centos-e2e-basic-test-release-1-6
/test metal3-centos-e2e-basic-test-release-1-7
/test metal3-centos-e2e-feature-test-main
/test metal3-centos-e2e-feature-test-release-1-5
/test metal3-centos-e2e-feature-test-release-1-6
/test metal3-centos-e2e-feature-test-release-1-7
/test metal3-centos-e2e-integration-test-main
/test metal3-centos-e2e-integration-test-release-1-5
/test metal3-centos-e2e-integration-test-release-1-6
/test metal3-dev-env-integration-test-centos-main
/test metal3-dev-env-integration-test-centos-release-1-5
/test metal3-dev-env-integration-test-centos-release-1-6
/test metal3-dev-env-integration-test-centos-release-1-7
/test metal3-dev-env-integration-test-ubuntu-release-1-5
/test metal3-dev-env-integration-test-ubuntu-release-1-6
/test metal3-dev-env-integration-test-ubuntu-release-1-7
/test metal3-e2e-1-26-1-27-upgrade-test-main
/test metal3-e2e-1-26-1-27-upgrade-test-release-1-5
/test metal3-e2e-1-26-1-27-upgrade-test-release-1-6
/test metal3-e2e-1-26-1-27-upgrade-test-release-1-7
/test metal3-e2e-1-27-1-28-upgrade-test-main
/test metal3-e2e-1-27-1-28-upgrade-test-release-1-5
/test metal3-e2e-1-27-1-28-upgrade-test-release-1-6
/test metal3-e2e-1-27-1-28-upgrade-test-release-1-7
/test metal3-e2e-1-28-1-29-upgrade-test-main
/test metal3-e2e-1-28-1-29-upgrade-test-release-1-5
/test metal3-e2e-1-28-1-29-upgrade-test-release-1-6
/test metal3-e2e-1-28-1-29-upgrade-test-release-1-7
/test metal3-e2e-clusterctl-upgrade-test-main
/test metal3-e2e-clusterctl-upgrade-test-release-1-5
/test metal3-e2e-clusterctl-upgrade-test-release-1-6
/test metal3-e2e-clusterctl-upgrade-test-release-1-7
/test metal3-ubuntu-e2e-basic-test-main
/test metal3-ubuntu-e2e-basic-test-release-1-6
/test metal3-ubuntu-e2e-basic-test-release-1-7
/test metal3-ubuntu-e2e-feature-test-main
/test metal3-ubuntu-e2e-feature-test-release-1-5
/test metal3-ubuntu-e2e-feature-test-release-1-6
/test metal3-ubuntu-e2e-feature-test-release-1-7
/test metal3-ubuntu-e2e-integration-test-main
/test metal3-ubuntu-e2e-integration-test-release-1-5
/test metal3-ubuntu-e2e-integration-test-release-1-6
/test metal3-ubuntu-e2e-integration-test-release-1-7

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

shellcheck

In response to this:

/test metal3-dev-env-integration-test-ubuntu-release-1.7
/test metal3-dev-env-integration-test-ubuntu-release-1.6

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.

kashifest · 2024-05-21T06:22:04Z

/test metal3-dev-env-integration-test-ubuntu-release-1-7
/test metal3-dev-env-integration-test-ubuntu-release-1-6

kashifest · 2024-05-21T10:05:32Z

/test metal3-dev-env-integration-test-ubuntu-release-1-6

mboukhalfa · 2024-05-21T12:51:06Z

/approve
/hold
We must check e2e tests https://github.com/metal3-io/cluster-api-provider-metal3/blob/95ac037d51099c05255993c53fbb068ec73bf996/scripts/environment.sh#L45

metal3-io-bot · 2024-05-21T12:51:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mboukhalfa

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:

~~OWNERS~~ [mboukhalfa]

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

kashifest · 2024-05-21T12:53:06Z

/hold We must check e2e tests https://github.com/metal3-io/cluster-api-provider-metal3/blob/95ac037d51099c05255993c53fbb068ec73bf996/scripts/environment.sh#L45

I am actually hardcoding it here , so exporting elsewhere shouldn't be an issue, we also have an export in project-infra.

mboukhalfa · 2024-05-21T12:56:33Z

/hold We must check e2e tests https://github.com/metal3-io/cluster-api-provider-metal3/blob/95ac037d51099c05255993c53fbb068ec73bf996/scripts/environment.sh#L45

I am actually hardcoding it here , so exporting elsewhere shouldn't be an issue, we also have an export in project-infra.

I am just thinking about when e2e export that in their environment var should not trigger any conflict later ?
If I understand correctly EPHEMERAL_CLUSTER="minikube" will be exported in devenv but for e2e still sees EPHEMERAL_CLUSTER="kind" is there any unwanted behavior when pivoting back or something ?

kashifest · 2024-05-21T13:18:27Z

I am just thinking about when e2e export that in their environment var should not trigger any conflict later ? If I understand correctly EPHEMERAL_CLUSTER="minikube" will be exported in devenv but for e2e still sees EPHEMERAL_CLUSTER="kind" is there any unwanted behavior when pivoting back or something ?

In that case lets run it for safety here,
/test metal3-dev-env-integration-test-ubuntu-release-1-6
/test metal3-ubuntu-e2e-integration-test-main

kashifest · 2024-05-22T05:35:12Z

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

kashifest · 2024-05-22T07:13:50Z

You were right @mboukhalfa

e2e wont pass with just only this I can see it is now trying to fetch ironic local container logs which it shouldnt , so e2e export is coming into play and needs to change as well if we go this way :

STEP: Fetch logs for container ironic @ 05/22/24 07:02:31.24
10:02:39  2024/05/22 07:02:31 exit status 1
10:02:39  Could not open /home/metal3ci/metal3/test/e2e/junit.e2e_suite.1.xml:
10:02:39  open /home/metal3ci/metal3/test/e2e/junit.e2e_suite.1.xml: no such file or directory

kashifest · 2024-05-22T07:16:38Z

/override metal3-ubuntu-e2e-integration-test-main , it would need a separate patch
/override metal3-centos-e2e-integration-test-release-1-7 centos is failing for unrelated reasons

metal3-io-bot · 2024-05-22T07:16:41Z

@kashifest: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

,
a
centos
failing
for
is
it
need
patch
reasons
separate
unrelated
would

Only the following failed contexts/checkruns were expected:

metal3-centos-e2e-integration-test-release-1-7
metal3-dev-env-integration-test-ubuntu-main
metal3-dev-env-integration-test-ubuntu-release-1-6
metal3-dev-env-integration-test-ubuntu-release-1-7
metal3-ubuntu-e2e-integration-test-main
shellcheck
tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override metal3-ubuntu-e2e-integration-test-main , it would need a separate patch
/override metal3-centos-e2e-integration-test-release-1-7 centos is failing for unrelated reasons

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.

kashifest · 2024-05-22T07:17:06Z

/override metal3-ubuntu-e2e-integration-test-main
/override metal3-centos-e2e-integration-test-release-1-7

metal3-io-bot · 2024-05-22T07:17:10Z

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

In response to this:

/override metal3-ubuntu-e2e-integration-test-main
/override metal3-centos-e2e-integration-test-release-1-7

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.

kashifest · 2024-05-22T07:47:04Z

@dtantsur @elfosardo PTAL here, after this change, plan is to run only ubuntu based tests as required in PRs, until we have a solution for centos based jobs. After this change, Ubuntu will run minikube as ephemeral cluster and run ironic as a k8s deployment (not as local containers as it is currently in kind cluster)

kashifest · 2024-05-22T09:46:06Z

/hold cancel

Sunnatillo · 2024-06-03T11:39:45Z

/hold

Do we still need this change?

kashifest · 2024-06-03T11:42:05Z

/hold

Do we still need this change?

We can discuss on it, I still think this is a better option given that:

We can reduce the tests being run
Not being dependant on CentOS packages which breaks more often compared to Ubuntu

kashifest · 2024-06-05T06:54:47Z

We can discuss on it, I still think this is a better option given that:

We can reduce the tests being run

Not being dependant on CentOS packages which breaks more often compared to Ubuntu

@Rozzii @lentzi90 @elfosardo how do you folks feel about this ?

lentzi90

I see no issues switching to use minikube throughout CI. However, I think we should keep it configurable.

lentzi90 · 2024-06-05T07:52:30Z

lib/common.sh

-else
-  export EPHEMERAL_CLUSTER="minikube"
-fi
+export EPHEMERAL_CLUSTER="minikube"


Suggested change

export EPHEMERAL_CLUSTER="minikube"

export EPHEMERAL_CLUSTER=${EPHEMERAL_CLUSTER:-"minikube"}

ow yeah, this I did intentionally here for the test because it gets dictated from different places like project-infra. CAPM3 etc

Rozzii · 2024-06-05T09:44:03Z

I was already proponent of this change and I still support it.
/lgtm

kashifest · 2024-06-10T11:49:46Z

I am guessing no one has any objections on this one, I will push an update on this PR and then we can take it in

dtantsur · 2024-06-10T14:40:57Z

test ironic as a k8s deployment instead of local containers.

You sold it to me here 👍🏽

elfosardo · 2024-06-11T13:11:36Z

definitely in favor of this!

tuminoid · 2024-06-11T13:15:59Z

If we keep it configurable and mean to support it, we need some periodic test at minimum to verify it keeps working. It is already sometimes breaking after we stopped requiring both ubuntu and centos e2e to be run on PRs.

Rozzii · 2024-06-12T06:17:46Z

@kashifest I think we can cancel the hold

tuminoid · 2024-06-12T06:27:27Z

If we keep it configurable and mean to support it, we need some periodic test at minimum to verify it keeps working. It is already sometimes breaking after we stopped requiring both ubuntu and centos e2e to be run on PRs.

Added #1418 for that. It must be implemented asap, if we merge this and plan to keep local ironic support, otherwise the feature will rot fast and be unsupportable.

metal3-io-bot · 2024-09-10T06:43:05Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale

kashifest · 2024-09-10T06:54:50Z

I might need to re-consider this PR.
/remove-lifecycle stale

metal3-io-bot requested review from dtantsur and mboukhalfa May 20, 2024 06:01

metal3-io-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 20, 2024

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 20, 2024

kashifest force-pushed the kashif/make-minikube-default-ephemeral-cluster branch from 1ee79f9 to df02ad3 Compare May 20, 2024 12:27

kashifest mentioned this pull request May 21, 2024

Use minikube in Ubuntu based tests #1405

Open

3 tasks

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 21, 2024

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 22, 2024

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 3, 2024

lentzi90 reviewed Jun 5, 2024

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 5, 2024

tuminoid mentioned this pull request Jun 12, 2024

Add periodic tests for local ironic scenario #1418

Open

metal3-io-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 10, 2024

metal3-io-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 10, 2024

	export EPHEMERAL_CLUSTER="minikube"
	export EPHEMERAL_CLUSTER=${EPHEMERAL_CLUSTER:-"minikube"}

Make minikube the default ephemeral cluster #1404

Are you sure you want to change the base?

Make minikube the default ephemeral cluster #1404

Conversation

kashifest commented May 20, 2024

kashifest commented May 20, 2024

kashifest commented May 20, 2024

kashifest commented May 20, 2024

mboukhalfa commented May 20, 2024

kashifest commented May 21, 2024

kashifest commented May 21, 2024

metal3-io-bot commented May 21, 2024

kashifest commented May 21, 2024

kashifest commented May 21, 2024

mboukhalfa commented May 21, 2024

metal3-io-bot commented May 21, 2024

kashifest commented May 21, 2024

mboukhalfa commented May 21, 2024 • edited Loading

kashifest commented May 21, 2024

kashifest commented May 22, 2024

kashifest commented May 22, 2024

kashifest commented May 22, 2024

metal3-io-bot commented May 22, 2024

kashifest commented May 22, 2024

metal3-io-bot commented May 22, 2024

kashifest commented May 22, 2024

kashifest commented May 22, 2024

Sunnatillo commented Jun 3, 2024

kashifest commented Jun 3, 2024 • edited Loading

kashifest commented Jun 5, 2024

lentzi90 left a comment

Choose a reason for hiding this comment

lentzi90 Jun 5, 2024

Choose a reason for hiding this comment

kashifest Jun 5, 2024

Choose a reason for hiding this comment

Rozzii commented Jun 5, 2024

kashifest commented Jun 10, 2024

dtantsur commented Jun 10, 2024

elfosardo commented Jun 11, 2024

tuminoid commented Jun 11, 2024

Rozzii commented Jun 12, 2024

tuminoid commented Jun 12, 2024

metal3-io-bot commented Sep 10, 2024

kashifest commented Sep 10, 2024

mboukhalfa commented May 21, 2024 •

edited

Loading

kashifest commented Jun 3, 2024 •

edited

Loading