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

Controller should set a proper field manager #84

Closed
4 tasks
erikgb opened this issue Sep 25, 2023 · 4 comments
Closed
4 tasks

Controller should set a proper field manager #84

erikgb opened this issue Sep 25, 2023 · 4 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@erikgb
Copy link
Contributor

erikgb commented Sep 25, 2023

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

  • Finish implementation of the issue
  • Test all functions
  • Have enough logs to trace activities
  • Notify developers of necessary actions
@zeroalphat
Copy link
Contributor

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

@ymmt2005 ymmt2005 added the bug Something isn't working label Sep 26, 2023
@ymmt2005
Copy link
Member

ymmt2005 commented Sep 27, 2023

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 ymmt2005 added enhancement New feature or request and removed bug Something isn't working labels Sep 27, 2023
@ymmt2005
Copy link
Member

Figured out that setting Go-http-client was actually a regression of controller-runtime.
kubernetes-sigs/controller-runtime#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"

@ymmt2005 ymmt2005 added the bug Something isn't working label Sep 27, 2023
@erikgb
Copy link
Contributor Author

erikgb commented Sep 27, 2023

@ymmt2005 Nice find! So we (unintentionally) fixed this in #88. 😄

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
Projects
None yet
Development

No branches or pull requests

3 participants