crossplane-contrib / provider-jet-rancher

Apache License 2.0
3 stars 4 forks source link

Use generic resource config plumbing to do common resource customizations #12

Closed sboschman closed 2 years ago

sboschman commented 2 years ago

Description of your changes

This PR introduces the same generic resource configuration setup as used by aws/gcp jet providers (config/providers.go and config/overrides.go). This should make it easier to do common resource customizations.

Common customizations introduced with this PR:

The api version of manual configured Cluster resource is bumped to v1alpha2, the other resources remain at v1alpha1. The new api reference can be inpsected here

I have:

How has this code been tested

ProviderConfig is unchanged and remains at v1alpha1.

Examples tested:

sboschman commented 2 years ago

@ytsarev hopefully yo have a few mins spare to look at it

sboschman commented 2 years ago

Thanks a lot for all the tests and clarifications, I think we can merge this one.

Additional changes to fix individual resources can be separate PRs imo, as it seems some resources are a little bit more 'complex' and require additional config.

For example, managing secrets throws CannotObserveExternalResource", "message": "cannot run refresh: refresh failed: Getting secret V2: Cluster ID and/or Secret V2 ID is nil

Or importing a cluster starts a reconcile like it owns the cluster "reason": "CannotUpdateExternalResource", "message": "cannot apply: apply failed: Bad response statusCode [422]. Status [422 Unprocessable Entity]. Body: [baseType=error, code=InvalidBodyContent, message=must have at least one node pool]

ytsarev commented 2 years ago

@sboschman could you please create an Issue for the last one?

sboschman commented 2 years ago

Did some digging into the cluster error, reason: https://github.com/rancher/terraform-provider-rancher2/issues/895

Cluster is imported with Crossplane into Rancher, so cluster lifecycle is managed outside Crossplane and Rancher. Rancher does place some annotations, which are returned by terraform (state). One of these annotations is an internal sync timestamp for Rancher itself. Crossplane detects config drift on this annotation and tries to resync the cluster resource. The terraform apply runs into the terraform-provider-rancher2 issue mentioned above.

So, problem 1 is that imported clusters can not be modified as a result of this bug. Even though changing the description (tested as well) and modifying annotations or labels should be allowed. Upstream problem imo, not something we can fix in the crossplane provider.

Problem 2 is the drift detection of labels/annotations which Crossplane does not control. Resetting the timestamp annotation by Crossplane to a previous value is a no go, regardless if this triggers the bug of problem 1. I guess the solution here should be ignoring (LateInitializer.IgnoredFields) all labels and annotations like xxx.cattle.io/yyy. As I don't want to hardcode labels/annotations I just happen to see on my cluster, it should be some sort of regex/wildcard match.

sboschman commented 2 years ago

opened #13 for this cluster issue

ytsarev commented 2 years ago

@sboschman cool, thanks, merging this one in