cybozu-go / accurate

Kubernetes controller for multi-tenancy. It propagates resources between namespaces accurately and allows tenant users to create/delete sub-namespaces.
https://cybozu-go.github.io/accurate/
Apache License 2.0
38 stars 5 forks source link

Controller should set a proper field manager #84

Closed erikgb closed 11 months ago

erikgb commented 11 months ago

What

Using Server-Side Apply in Kubernetes requires that controllers set a non-dummy field manager. Below is an extract of a namespace created by the accurate controller in one of our clusters. As you can see, the field manager for fields set by accurate is marked with Go-http-client as the manager. This will cause issues if another controller does not set a non-default manager value. We also use managedFields a lot when debugging issues in our clusters.

Accurate controller should set a field manager value when creating/updating API resources. Suggested value accurate.

kind: Namespace
apiVersion: v1
metadata:
  name: erikbo-egb-test
  uid: f98a73f3-04dc-4abc-b673-f2b6970ca318
  resourceVersion: '698120538'
  creationTimestamp: '2023-09-25T14:20:40Z'
  labels:
    accurate.cybozu.com/parent: erikbo
    app.kubernetes.io/created-by: accurate
    kubernetes.io/metadata.name: erikbo-egb-test
  managedFields:
    - manager: Go-http-client
      operation: Update
      apiVersion: v1
      time: '2023-09-25T14:20:40Z'
      fieldsType: FieldsV1
      fieldsV1:
        'f:metadata':
          'f:labels':
            .: {}
            'f:accurate.cybozu.com/parent': {}
            'f:app.kubernetes.io/created-by': {}
            'f:kubernetes.io/metadata.name': {}

How

Describe how to address the issue.

Checklist

zeroalphat commented 11 months ago

@erikgb As your feedback, it looks like it should be fixed. We will fix it.

ymmt2005 commented 11 months ago

Accurate does not use server-side apply yet. operation: Update indicates that the change was made by normal Update operation rather than SSA.

It might be better to set an appropriate FieldManager, but not setting it is not a bug as we are not using SSA.

ymmt2005 commented 11 months ago

Figured out that setting Go-http-client was actually a regression of controller-runtime. https://github.com/kubernetes-sigs/controller-runtime/pull/2435

This is fixed in controller-runtime v0.16, so Accurate will set the field manager to accurate-controller with the next release.

I checked the current code and saw:

$ kubectl -n sub1 get secrets -o yaml --show-managed-fields
apiVersion: v1
items:
- apiVersion: v1
  data:
    foo: YmFy
  kind: Secret
  metadata:
    annotations:
      accurate.cybozu.com/from: root1
      accurate.cybozu.com/propagate: update
    creationTimestamp: "2023-09-27T13:47:06Z"
    labels:
      app.kubernetes.io/created-by: accurate
    managedFields:
    - apiVersion: v1
      fieldsType: FieldsV1
      fieldsV1:
        f:data:
          .: {}
          f:foo: {}
        f:metadata:
          f:annotations:
            .: {}
            f:accurate.cybozu.com/from: {}
            f:accurate.cybozu.com/propagate: {}
          f:labels:
            .: {}
            f:app.kubernetes.io/created-by: {}
        f:type: {}
      manager: accurate-controller
      operation: Update
      time: "2023-09-27T13:47:06Z"
erikgb commented 11 months ago

@ymmt2005 Nice find! So we (unintentionally) fixed this in https://github.com/cybozu-go/accurate/pull/88. 😄