-
Notifications
You must be signed in to change notification settings - Fork 119
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
Make minikube the default ephemeral cluster #1404
base: main
Are you sure you want to change the base?
Make minikube the default ephemeral cluster #1404
Conversation
/hold |
/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>
1ee79f9
to
df02ad3
Compare
/test metal3-dev-env-integration-test-ubuntu-main |
Can you create an issue to keep track and add TODOs once centos back to normal |
|
/test metal3-dev-env-integration-test-ubuntu-release-1.7 |
@kashifest: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
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. |
/test metal3-dev-env-integration-test-ubuntu-release-1-7 |
/test metal3-dev-env-integration-test-ubuntu-release-1-6 |
/approve |
[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:
Approvers can indicate their approval by writing |
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 ? |
In that case lets run it for safety here, |
/test metal3-ubuntu-e2e-integration-test-main |
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 :
|
/override metal3-ubuntu-e2e-integration-test-main , it would need a separate patch |
@kashifest: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
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:
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. |
/override metal3-ubuntu-e2e-integration-test-main |
@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:
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. |
@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) |
/hold cancel |
/hold Do we still need this change? |
We can discuss on it, I still think this is a better option given that:
|
@Rozzii @lentzi90 @elfosardo how do you folks feel about this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no issues switching to use minikube throughout CI. However, I think we should keep it configurable.
else | ||
export EPHEMERAL_CLUSTER="minikube" | ||
fi | ||
export EPHEMERAL_CLUSTER="minikube" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export EPHEMERAL_CLUSTER="minikube" | |
export EPHEMERAL_CLUSTER=${EPHEMERAL_CLUSTER:-"minikube"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ow yeah, this I did intentionally here for the test because it gets dictated from different places like project-infra. CAPM3 etc
I was already proponent of this change and I still support it. |
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 |
You sold it to me here 👍🏽 |
definitely in favor of this! |
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. |
@kashifest I think we can cancel the hold |
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. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with /lifecycle stale |
I might need to re-consider this PR. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with /lifecycle stale |
/remove-lifecycle stale |
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.