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.

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

Always get labels from Bundle when creating a BundleDeployment #1428

Merged
merged 3 commits into from Mar 28, 2023

Conversation

raulcabello
Copy link
Contributor

@raulcabello raulcabello commented Mar 21, 2023

When a label is modified in the fleet.yaml is also modified in the Bundle, but no in the BundleDeployment

BundleDeployment labels are not being updated when changed in the fleet.yaml. This is because we just get the labels from the Bundle the first time the BundleDeployment is created, this is done in the resetDeployment method.

This PR always get the labels from the Bundle when creating a BundleDeployment

BundleDeployment patch was failing in the integration tests because wrangler was using application/strategic-merge-patch+json strategy for the patch. Then I was getting the error:

time="2023-03-24T17:04:02+01:00" level=error msg="error syncing 'test-0e60e6e/name': handler bundle: failed to update test-0e60e6e/name fleet.cattle.io/v1alpha1, Kind=BundleDeployment for bundle test-0e60e6e/name: the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml, requeuing"

This was happening because I registered the Bundle and BundleDeployment in the client, then wrangler was choosing this strategy here. I fixed this by not registering the Bundle and BundleDeployment in the client as they are not needed, then I did the same for the BundleDeployment integration tests here in order to prevent this error in the future.

refers to #1153

BundleDeployment labels were not being updated when changed in the fleet.yaml
Always get the labels from the Bundle when creating a BundleDeployment

Signed-off-by: raul <raul.cabello@suse.com>
Add a test case to cover the scenario where a Bundle label is changed,
and this changed is applied to the relevant BundleDeployment.

Signed-off-by: raul <raul.cabello@suse.com>
It is not needed as we always use wrangler controllers to create or update resources (such as BundleDeployments)
This was causing wrangler to use application/strategic-merge-patch+json when doing a patch, and this was causing the patch to fail.

Signed-off-by: raul <raul.cabello@suse.com>
@raulcabello raulcabello marked this pull request as ready for review March 24, 2023 16:12
@raulcabello raulcabello requested a review from a team as a code owner March 24, 2023 16:12
Copy link
Member

@manno manno left a comment

Choose a reason for hiding this comment

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

👍

@manno manno merged commit c12eace into rancher:master Mar 28, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants