argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
15.08k stars 3.2k forks source link

support Field Management (managedFields) #1880

Open dorsany opened 4 years ago

dorsany commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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?

alexec commented 4 years ago

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?

scodeman commented 4 years ago

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 commented 4 years ago

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 ?

elim19 commented 1 year ago

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 commented 1 year ago

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