-
Notifications
You must be signed in to change notification settings - Fork 294
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
🌱 Refactor test data to allow multiple versions #2250
🌱 Refactor test data to allow multiple versions #2250
Conversation
/test |
@killianmuldoon: The
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 pull-cluster-api-provider-vsphere-e2e-full-main |
2c3107b
to
804c474
Compare
/test pull-cluster-api-provider-vsphere-e2e-full-main |
804c474
to
cc9a634
Compare
/test pull-cluster-api-provider-vsphere-e2e-full-main |
cc9a634
to
85aa4ca
Compare
Only one failure in the full-e2e run which I think must have been a flake so this is looking ready for review IMO. |
85aa4ca
to
b904ad7
Compare
/test pull-cluster-api-provider-vsphere-e2e-full-main |
b904ad7
to
11dfc27
Compare
/test pull-cluster-api-provider-vsphere-e2e-full-main |
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.
Just nits
/lgtm
779d126
to
1c70381
Compare
/test pull-cluster-api-provider-vsphere-e2e-full-main |
Makefile
Outdated
@@ -595,8 +597,12 @@ release-manifests: $(BUILD_DIR) $(MANIFEST_DIR) $(KUSTOMIZE) $(STAGE)-flavors ## | |||
cp metadata.yaml $(MANIFEST_DIR)/metadata.yaml | |||
|
|||
.PHONY: release-flavors ## Create release flavor manifests | |||
release-flavors: $(RELEASE_DIR) | |||
$(MAKE) generate-flavors FLAVOR_DIR=$(RELEASE_DIR) | |||
release-flavors: $(KUSTOMIZE) $(addprefix release-flavors-, main) |
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 think this breaks our release GitHub action
release-manifests-all > release-manifests > release-flavors and then the action picks up stuff from the out folder
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.
You're dead right - added a specific target for generating the flavors here, but still to the out
directory.
I'm wondering if we need to keep or if we actually use dev-flavors
which copies a manifest to the overrides
folder in the clusterctl repo, but I think we can come back to cleaning up the dev workflow once we're all more experienced with it.
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.
Yup. I think I would stay away from dev-flavors and that overrides dir :D
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.
Maybe let's preserve whatever that dev workflow is (that we're not really using because of tilt) and have our own thing for e2e
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
1c70381
to
96f02b7
Compare
/lgtm /test pull-cluster-api-provider-vsphere-e2e-full-main /hold |
LGTM label has been added. Git tree hash: 1d63fb7c538d93f6809ef407e249f6c2af4a0a91
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
Up to you how you want to cherry-pick this stuff. (every PR or once we have the final version on main working) |
Seems like a flake /retest |
/cherry-pick release-1.8 |
@killianmuldoon: once the present PR merges, I will cherry-pick it on top of release-1.8 in a new PR and assign it to you. 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. |
I think the versions are off for release-1.8 even if the cherry-pick goes through |
/hold cancel |
e2e now passing - I'll do the cherry picks manually |
/retest |
@killianmuldoon: #2250 failed to apply on top of branch "release-1.8":
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. |
This PR restructures the test data folder for e2e tests to make it more friendly to holding multiple versions at a time.
Part of #2251