argo-workflows icon indicating copy to clipboard operation
argo-workflows copied to clipboard

support Field Management (managedFields)

Open dorsany opened this issue 5 years ago • 7 comments
trafficstars

Summary

In my k8s cluster I have the Field Management turned on for audit purposes. The Field Management will add another key called managedFields under metadata.managedFields and this managedFields can be more than depth (10) which is the limit in argo workflow for WorkflowTemplate

example how the WorkflowTemplate it looks:

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  creationTimestamp: "2019-12-19T18:29:03Z"
  generation: 1
  managedFields:
  - apiVersion: argoproj.io/v1alpha1
    fields:
      f:spec:
        f:templates: {}
    manager: Go-http-client
    operation: Update
    time: "2019-12-19T18:29:03Z"
  name: workflow-template-core-example
  namespace: default
  resourceVersion: "2074547"
  selfLink: /apis/argoproj.io/v1alpha1/namespaces/default/workflowtemplates/workflow-template-core-example
  uid: 6eb6376c-228d-11ea-b8e5-7085c278ad76
spec:
  templates:
  - args:
    - hello
    command:
    - echo
    container: null
    image: busybox
    imagePullPolicy: IfNotPresent
    name: core-example-migrator-pod-template
    nodeSelector:
      backend: "true"

and I'm getting the error:

time="2019-12-19T18:29:21Z" level=info msg="Updated phase Running -> Failed" namespace=default workflow=workflow-core-q8wn1
time="2019-12-19T18:29:21Z" level=info msg="Updated message  -> invalid spec: templates.workflow-template-core.tasks.core-example-migrator template reference exceeded max depth (10)" namespace=default workflow=workflow-core-q8wn1

What change you think needs making: just ignore this key: metadata.managedFields

Motivation

audit purposes.

Proposal

How do you think this should be implemented? just ignore the key metadata.managedFields if this key exist, the question is where (context.go?validate.go?) or increase max depth default value to be more than 10

more on: https://kubernetes.io/docs/reference/using-api/api-concepts/#field-management

dorsany avatar Dec 19 '19 18:12 dorsany

Hi @alexec , Can you please review this issue? I can create a PR to fix this by ignore the managed fields key or increase the max depth , but I need your guide of how do you want to implement the solution

dorsany avatar Jan 19 '20 09:01 dorsany

Hi @alexec , I can't work with WorkflowTemplate until I will find a solution for my max depth issue. Can you please review it and let my know which solution is better?

dorsany avatar Feb 01 '20 10:02 dorsany

hi @dorsany, I'm afraid I don't know much about this managed fields - is this something you'd be able to look into and suggest a solution please?

alexec avatar Feb 03 '20 17:02 alexec

Is there a particular reason the depth is set to 10 max ? This appears quite limiting. I am facing the same Internal Server Error: template reference exceeded max depth (10) issue just by trying to define a basic Service amongst my ressource templates. Ex:

---
metadata:
  name: test
  namespace: argo
  labels:
    example: 'true'
spec:
  entrypoint: basic-service
  templates:
  - name: basic-service
    ressources:
      action: create
      manifest:
        apiVersion: v1
        kind: Service
        metadata:
          name: basic-service
        spec:
          selector:
            app: test
          ports:
          - protocol: TCP
            port: 8080
            targetPort: 8080

scodeman avatar Sep 22 '20 16:09 scodeman

I could see that https://github.com/argoproj/argo/pull/1553 had also a proposal for URLs as input for manifest. Could it help if such feature was available ?

scodeman avatar Sep 22 '20 16:09 scodeman

I don't think https://github.com/argoproj/argo-workflows/pull/1553 has anything to do with managedFields, @alexec , currently it will add all nodes to the managedFields, so the more nodes workflow have, the larger this field is. etcd has a size limit of 1.5m, this will greatly make workflow larger, is there anything we can do about it?

elim19 avatar Feb 01 '23 00:02 elim19

https://kubernetes.io/docs/reference/using-api/server-side-apply/#clearing-managedfields It can be cleared, will workflow controller consider doing this? @alexec

elim19 avatar Feb 01 '23 01:02 elim19