Server-side dry-run fails for bucket with lifecycle rule

[yhrn]

Describe the bug kubectl diff --server-side and kpt live preview --server-side both fails for an unchanged bucket with lifecyleRule in the spec. kubectl outputs:

W0820 21:24:20.343934 2785154 diff.go:523] Object (storage.cnrm.cloud.google.com/v1beta1, Kind=StorageBucket: some-bucket) keeps changing, diffing without lock
Error from server (Conflict): Apply failed with 1 conflict: conflict with "before-first-apply" using storage.cnrm.cloud.google.com/v1beta1: .spec.lifecycleRule

Note that kubectl apply --dry-run=server works as expected.

ConfigConnector Version 1.17.0

To Reproduce

1. kubectl apply -f file-with-bucket
2. kubectl diff --server-side -f file-with-bucket

If the lifecycleRule is removed the diff works but outputs the, somewhat confusing, diff included further down.

YAML snippets:

apiVersion: storage.cnrm.cloud.google.com/v1beta1
kind: StorageBucket
metadata:
  name: some-bucket
  namespace: some-namespace
  annotations:
    cnrm.cloud.google.com/force-destroy: "true"
spec:
  storageClass: MULTI_REGIONAL
  location: EU
  bucketPolicyOnly: true
  lifecycleRule:
  - action:
      type: Delete
    condition:
      age: 7

diff:

diff -u -N /tmp/LIVE-850095419/storage.cnrm... /tmp/MERGED-345744222/storage.cnrm...
--- /tmp/LIVE-850095419/storage.cnrm... 2020-08-20 21:40:28.497882896 +0200
+++ /tmp/MERGED-345744222/storage.cnrm...   2020-08-20 21:40:28.765885723 +0200
@@ -12,6 +12,42 @@
   - cnrm.cloud.google.com/finalizer
   - cnrm.cloud.google.com/deletion-defender
   generation: 3
+  managedFields:
+  - apiVersion: storage.cnrm.cloud.google.com/v1beta1
+    fieldsType: FieldsV1
+    fieldsV1:
+      f:metadata:
+        f:annotations:
+          f:cnrm.cloud.google.com/force-destroy: {}
+      f:spec:
+        f:bucketPolicyOnly: {}
+        f:location: {}
+        f:storageClass: {}
+    manager: kubectl
+    operation: Apply
+    time: "2020-08-20T19:40:28Z"
+  - apiVersion: storage.cnrm.cloud.google.com/v1beta1
+    fieldsType: FieldsV1
+    fieldsV1:
+      f:metadata:
+        f:annotations:
+          f:cnrm.cloud.google.com/force-destroy: {}
+          f:cnrm.cloud.google.com/management-conflict-prevention-policy: {}
+          f:cnrm.cloud.google.com/project-id: {}
+          f:kubectl.kubernetes.io/last-applied-configuration: {}
+        f:finalizers:
+          v:"cnrm.cloud.google.com/deletion-defender": {}
+          v:"cnrm.cloud.google.com/finalizer": {}
+      f:spec:
+        f:bucketPolicyOnly: {}
+        f:location: {}
+        f:storageClass: {}
+      f:status:
+        f:conditions: {}
+        f:selfLink: {}
+        f:url: {}
+    manager: before-first-apply
+    operation: Update
   name: some-bucket
   namespace: some-namespace
   resourceVersion: "54658400"

[jcanseco]

Hi Mattias. The managedFields block in the diff is simply metadata for server-side apply (more info). I'm guessing what's happening here is the resource did not have the server-side metadata before, and is now being retroactively added by K8s. Prior to K8s 1.18, server-side apply metadata is not included by default and is instead retroactively added on the first server-side apply operation.

kubectl diff --server-side and kpt live preview --server-side both fails for an unchanged bucket with lifecyleRule in the spec

Regarding this issue, can you please confirm if KCC defaulted any values within spec.lifecycleRule after it was first applied? In other words, is there a difference between the YAML and the resource state on the API server? What you're seeing is probably an issue with list merging, but we'd like to confirm first.

[yhrn]

Hi @jcanseco,

Yes it seems to add condition.withState: ANY

[jcanseco]

Gotcha. This seems like an atomic list issue then. Simply put, we still treat lists as atomic just like before. That is, the list that you apply is what it is, and KCC then sets fields within the list that are defaulted by GCP afterwards.

A consequence of this behavior is that KCC's server-side apply manager must take ownership of the whole list (the technical details as to why are a bit out of scope for this discussion), resulting in conflicts whenever a later server-side apply tries to update the list (which would always be the case if GCP defaulted any values within the list, causing the list to be different from your YAML).

So currently, the recommendation for when a server-side apply involves a list diff is to force the apply (kubectl apply --server-side --force-conflicts=true). This will effectively replicate the client-side apply behavior for lists which you're more accustomed to.

In the future, we aim to support list merging via server-side apply so that you would not have to force an apply unless necessary (i.e. when you truly want to modify a field owned by another manager rather than today where a conflict is caused by your YAML simply not including a field owned by the KCC manager).

[yhrn]

Sounds good. Will you keep this issue open until you support list merging via server-side apply or do you want to track that somewhere else?

[jcanseco]

We can keep this issue open for now since we don't have an issue which specifically tracks list merging. Thanks.

[robertomlsoares]

Hi @jcanseco! Are there any updates on this issue? Specifically list merging for SSA.

[diviner524]

@robertomlsoares We do have a new feature which might be helpful in similar situations. Simply put, if cnrm.cloud.google.com/state-into-spec: "absent" is specified, Config Connector will not default any unspecified values back to the spec. Please give it a try and see if it works for you.

Regarding list merging feature, we don't have a solid ETA at this moment.

[robertomlsoares]

Hi, thanks for the suggestion! I was more interested to know about list merging for SSA. We have a possible migration to SSA in our roadmap coming up, and I wanted to make sure Config Connector plays well with it. Would you say that it's not recommended for Config Connector users to migrate to SSA, then?

[diviner524]

@robertomlsoares By "migrate" do you mean enable server-side apply in your K8s cluster?

Config Connector supports server-side apply (except for list fields, which are treated atomically), and most GKE + Config Connector users will have server-side apply enabled by default when they are using Config Connector. However list merging is not supported, we only support managing list fields externally through the state-into-spec annotation mentioned earlier.

You can find more details in related links below:

https://cloud.google.com/config-connector/docs/concepts/managing-fields-externally https://cloud.google.com/config-connector/docs/concepts/ignore-unspecified-fields#manage_unspecified_fields_in_a_list_externally