[go: nahoru, domu]

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

Handling provider-added fields to maps and slices #120

Open
hasheddan opened this issue Feb 21, 2020 · 2 comments
Open

Handling provider-added fields to maps and slices #120

hasheddan opened this issue Feb 21, 2020 · 2 comments
Labels
bug Something isn't working enhancement New feature or request wontfix This will not be worked on

Comments

@hasheddan
Copy link
Member

What problem are you facing?

Currently, if there is a configurable slice of fields on a managed resource, we check to make sure that the external resource and the managed resource have the same elements of that slice. One example of this is with labels. We want to add all labels that a user provides, and remove any that get set outside the context of Crossplane. However, this becomes an issue if the provider itself adds some elements to the slice. For example, when gVisor is enabled on a GKE NodePool, GCP adds the following to labels and taints:

                DiskType:  "pd-standard",                                                                                                                                                                   
                ImageType: "COS_CONTAINERD",                                                                                                                                                                
                Labels: map[string]string{                                                                                                                                                                  
+                       "sandbox.gke.io/runtime": "gvisor",                                                                                                                                                 
                        "some-label":            "true",                                                                                                                                                   
                },
                LocalSsdCount: 0,
                MachineType:   "n1-standard-2",
                ... // 6 identical fields
                ShieldedInstanceConfig: &container.ShieldedInstanceConfig{EnableIntegrityMonitoring: true, EnableSecureBoot: true},                                                                        
                Tags:                   nil,
                Taints: []*container.NodeTaint{
                        &{Effect: "NO_SCHEDULE", Key: "some-pods", Value: "false"},
+                       &{Effect: "NO_SCHEDULE", Key: "sandbox.gke.io/runtime", Value: "gvisor"},
                },
                WorkloadMetadataConfig: nil,
                ForceSendFields:        nil,
                NullFields:             nil,

When we compare the external resource and the managed resource, we see that there is a diff and attempt to remove the additional fields. This can cause a continuous update loop as we remove the fields and GCP keeps adding them back.

How could Crossplane help solve your problem?

The solution here will be more of a design decision than a bug fix. One could argue that the controller is acting correctly here and a user should provide these labels and taints in their spec if they want Crossplane to not try and remove them. However, that results in a rather poor user experience.

The most straight-forward solution is that we late initialize any slice values that we deem "mergeable" with values the cloud provider adds. This would be in accordance with the Kubernetes late initialization patterns. However, this would necessitate some sort of change in our managed reconciler because we currently late initialize before updating an external resource. This leads to the situation where a user:

  1. Delete an element from a slice
  2. The controller reconciles the object, notices the element is present on the cloud provider, so late initializes
  3. On the next reconcile, the controller now sees that element present again (because of late initialization) and does not detect a diff so will not try to delete it

Another option is to manually implement logic that strategically ignores some field updates (such as the gVisor ones above), but that does not scale well across all API types.

@muvaf
Copy link
Member
muvaf commented Feb 22, 2020

Delete an element.

For maps, we can have users do something like "key-i-want-to-delete": "-" for the keys that they want to delete, a little bit similar to kubectl label pods foo bar-. Unless a key has a value like this, we merge all.

Slices are harder, especually if provider wants to add an element at an arbitrary point of the lifecycle of the resource. Maybe go with manual ignore for slices?

@stale
Copy link
stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants