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

workflow-controller-configmap misses the config key #2504

Closed
makeItFuckingSustainable opened this issue Mar 24, 2020 · 4 comments · Fixed by #2505
Closed

workflow-controller-configmap misses the config key #2504

makeItFuckingSustainable opened this issue Mar 24, 2020 · 4 comments · Fixed by #2505
Labels

Comments

@makeItFuckingSustainable

What happened:

I applied a minimal version of the workflow-controller-configmap from https://raw.githubusercontent.com/argoproj/argo/master/docs/workflow-controller-configmap.yaml:

The workflow-controller logs gave:

time="2020-03-24T14:20:54Z" level=warning msg="ConfigMap 'workflow-controller-configmap' does not have key 'config'"
time="2020-03-24T14:20:54Z" level=info msg="workflow controller configuration from workflow-controller-configmap:\n"

No default artifact registry was configured.

What you expected to happen:

I expected to see log entries in the workflow-controller of the form

time="2020-03-24T14:42:08Z" level=info msg="Detected ConfigMap update. Updating the controller config."
time="2020-03-24T14:42:08Z" level=info msg="workflow controller configuration from workflow-controller-configmap:\nartifactRepository: \n  archiveLogs: true\n  s3:\n    endpoint: s3.amazonaws.com\n    bucket: my-bucket\n    region: us-west-2\n    insecure: false\n    accessKeySecret:\n      name: my-s3-credentials\n      key: accessKey\n    secretKeySecret:\n      name: my-s3-credentials\n      key: secretKey\n"

How to reproduce it (as minimally and precisely as possible):

Prerequisites:

  • Assuming that argo workflows is installed into the argo namespace.
  • Argo workflow controller uses image argoproj/workflow-controller:v2.6.3
cat > test_config.yaml <<<EOF
apiVersion: v1
kind: ConfigMap
metadata:
  name: workflow-controller-configmap
  namespace: argo
data:
  artifactRepository: |
    archiveLogs: true
    s3:
      endpoint: s3.amazonaws.com
      bucket: my-bucket
      region: us-west-2
      insecure: false
      accessKeySecret:
        name: my-s3-credentials
        key: accessKey
      secretKeySecret:
        name: my-s3-credentials
        key: secretKey
EOF

kubectl apply -f test_config.yaml
kubectl logs -n argo $(kubectl get pods -l app=workflow-controller -n argo -o name)

Anything else we need to know?:

proposed solution:

Add a missing sub-key config into the workflow-controller-configmap as

apiVersion: v1
kind: ConfigMap
metadata:
  name: workflow-controller-configmap
  namespace: argo
data:
  config: |
    artifactRepository: 
...

Note, that the remaining pipe symbols "|" have to be taken out (e.g. after artifactRepository).

Environment:

  • Argo version: v2.6.3 (in cluster)
  • Argo version: v2.6.2 (cli)
  • Kubernetes version : v1.11.10

Other debugging information (if applicable):

  • workflow-controller logs:
kubectl logs -n argo $(kubectl get pods -l app=workflow-controller -n argo -o name)
time="2020-03-24T14:20:54Z" level=warning msg="ConfigMap 'workflow-controller-configmap' does not have key 'config'"
time="2020-03-24T14:20:54Z" level=info msg="workflow controller configuration from workflow-controller-configmap:\n"

Message from the maintainers:

If you are impacted by this bug please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

@simster7
Copy link
Member

Hi @makeItFuckingSustainable, we are upgrading the way we structure our configuration maps and the changes you linked have not been in a release yet.

Could you take a look at this version of the file? It should work for you

@simster7
Copy link
Member

@alexec We need to make sure we have both versions of the config map in our docs, so that users who are not yet on 2.7 don't run into issues.

@makeItFuckingSustainable
Copy link
Author

Hi @simster7, my proposed solution seems to match that. Thanks for getting back to me so quickly.

@simster7
Copy link
Member

Thanks for bringing this up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants