Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Exclude resources if we cannot find their scope #1943

Merged
merged 1 commit into from
Apr 16, 2019

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Apr 16, 2019

Fixes #1941

This is aiming at custom resources whose scope is unknown because its CRD is included in a helm chart Flux won't inspect (since it's the helm operator's responsibility to do it).

This allows Flux to move forward and create the HelmRelease supplying the CRD. In a subsequent sync, once we know its scope, the custom resources will be created.

I was on the fence on whether I should revert #1876 but I decided against it, since it's not a lot of code (mostly reusable test-code) and it's less confusing for the user to obtain the custom resource scope locally if we have it.

@2opremio 2opremio requested a review from hiddeco April 16, 2019 01:10
@2opremio 2opremio force-pushed the 1941-fix-CRD-syncing branch 2 times, most recently from e144eb8 to 17cee20 Compare April 16, 2019 01:18
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

💯

Do we want to make a patch release out of this?

This is aiming at custom resources whose scope is unknown because
its CRD is included in a helm chart Flux won't inspect (since it's
the helm operator's responsibility to do it).

This allows Flux to move forward and create the `HelmRelease` supplying the
CRD. In a subsequent sync, once we know its scope, the custom resources will be
created.
@2opremio 2opremio force-pushed the 1941-fix-CRD-syncing branch from 17cee20 to 6f2eba2 Compare April 16, 2019 10:46
@2opremio
Copy link
Contributor Author

Do we want to make a patch release out of this?

Yeah, I think we do.

@brantb Before we merge, can you test this image to double-check the problem is gone for you? quay.io/fons/flux:1941-fix-crd-syncing-6f2eba28

@brantb
Copy link
Contributor

brantb commented Apr 16, 2019

@2opremio
It's working!

@2opremio
Copy link
Contributor Author

Heheh, great

@2opremio 2opremio merged commit fec2e9f into fluxcd:master Apr 16, 2019
@2opremio 2opremio deleted the 1941-fix-CRD-syncing branch April 16, 2019 19:35
@squaremo squaremo added this to the v1.12.1 milestone Apr 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cope when syncing a helm-provided CRD and resources using the CRD
4 participants