-
Notifications
You must be signed in to change notification settings - Fork 101
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
🌱 E2E: Install independent Metal3 IPAM after upgrade #2382
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
f25f534
to
ef7f5cc
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
You are using I created a PR to change it to Metal3 because that is a better and more correct name. |
/test metal3-e2e-clusterctl-upgrade-test-main |
let's change the provider name here too:
|
ef7f5cc
to
03dd3e7
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
2 similar comments
/test metal3-e2e-clusterctl-upgrade-test-main |
/test metal3-e2e-clusterctl-upgrade-test-main |
03dd3e7
to
3612a78
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
It is still complaining about target provider components for IPAM, even with the new name. I don't see open PRs anymore in any repo related to this, besides this one. What are we missing? |
There is something wrong with the clusterctl-config. It doesn't have the Metal3 IPAM provider listed. 🤔 |
We can move the following code to and try again. But I am not sure why dev-env is not setting it. |
Dev-env is setting it but possibly not in the place that is used by this test. There could also be something with the overrides structure that need to be adjusted. |
This is from earlier in the test. Does it look correct? I'm getting very confused with all the overrides and
|
this seems fine to me, here |
It is probably correct. I'm just getting confused by at all. |
yes we need to add there, I forgot about that. For e2e it gets it from there. |
3612a78
to
5dd8f6e
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
I don't get it. We have proper tags there. What is the issue? 🤔 |
It is looking for exactly v1.9 which we don't have? We have v1.9.0 |
I don't think so. We use this same syntax everywhere to get the latest patch version for that minor version. |
test/e2e/config/e2e_conf.yaml
Outdated
- name: metal3 | ||
type: IPAMProvider | ||
versions: | ||
- name: "{go://github.com/metal3-io/[email protected]}" |
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.
Here, IPAM is slightly different from other providers because we want to test the main
branch rather than version 1.9.
However, I'm wondering how that would work. We can use v1.10.99
instead of {go://github.com/metal3-io/[email protected]}
, but how will we obtain the YAML resources for the main
branch? Should we push them somewhere during our image-building process? As far as I know, we only push images to quay.
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.
Oh we have them in the overrides folder:
Potential override file searchFile="/home/metal3ci/.config/cluster-api/overrides/ipam-metal3/v1.10.99/metadata.yaml" provider="ipam-metal3" version="v1.10.99"
Using override="metadata.yaml" provider="ipam-metal3" version="v1.10.99"
We used them in dev-env, we can use these ones.
test/e2e/upgrade_clusterctl_test.go
Outdated
ClusterctlConfigPath: clusterctlConfigPath, | ||
KubeconfigPath: clusterProxy.GetKubeconfigPath(), | ||
LogFolder: ipamDeployLogFolder, | ||
IPAMProviders: []string{Metal3ipamProviderName}, |
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.
IPAMProviders: []string{Metal3ipamProviderName}, | |
IPAMProviders: []string{fmt.Sprintf(Metal3ipamProviderName, ipamRelease)}, |
We need to give the version here.
This is failing because it attempts to find the |
5dd8f6e
to
30f2e5f
Compare
Good catch! We will need to do it with an override then I guess? |
30f2e5f
to
70bd8c9
Compare
yes |
I wonder how much difference there really is in the manifests. Going to do a test run with just added metadata and see how it goes. |
70bd8c9
to
13885d1
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
this is probably still going to fail because the code that is resolving these markers: {go://github.com/metal3-io/[email protected]} does not check |
13885d1
to
09ad410
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
09ad410
to
f62fdf8
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
f62fdf8
to
3b3c78b
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
The Metal3 IPAM was previously bundled with CAPM3. Now we deploy it separately as a CAPI IPAM provider. In clusterctl upgrade tests going from a version where IPAM is bundled, to a version where it is not, we must install it after the upgrade. This commit adds a post upgrade hook to the clusterctl upgrade tests that installs the Metal3 IPAM. Signed-off-by: Lennart Jern <[email protected]>
3b3c78b
to
d97c33f
Compare
/test metal3-e2e-clusterctl-upgrade-test-main |
@lentzi90: The following test failed, say
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. |
@lentzi90: The following test failed, say
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. |
What this PR does / why we need it:
The Metal3 IPAM was previously bundled with CAPM3. Now we deploy it
separately as a CAPI IPAM provider. In clusterctl upgrade tests going
from a version where IPAM is bundled, to a version where it is not, we
must install it after the upgrade.
This commit adds a post upgrade hook to the clusterctl upgrade tests
that installs the Metal3 IPAM.
NOTE: This also includes #2380 we can merge that first or we can close it. I just wanted to make sure I don't hit that issue when testing the upgrade.
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 #