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

fix!: Only get executor plugins in workflow namespace. Fixes #12708 #12724

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jswxstw
Copy link
Member

@jswxstw jswxstw commented Mar 1, 2024

Fixes #12708

Motivation

Controller plugins are loaded by default, so controller service account will be accessed if AutomountServiceAccountToken is true, which cause the bug in #12708 .

Modifications

Only load plugins in user‘s workflow namespace.

Verification

local test

@jswxstw jswxstw force-pushed the fix-12708 branch 2 times, most recently from 794e8dc to fa939bc Compare March 4, 2024 02:29
@shuangkun
Copy link
Member

I think this is reasonable. Can you add some tests, such as deploying a plugin under default, another plugin under controller, and checking the sidecar of the pod under default.

@jswxstw
Copy link
Member Author

jswxstw commented Apr 2, 2024

I've tested it locally.
There are two executor plugins in namespace argo and khaos-workflow.

# kubectl get cm -A -l workflows.argoproj.io/configmap-type=ExecutorPlugin
NAMESPACE        NAME                          DATA   AGE
khaos-workflow   khaos-steps-executor-plugin   2      21m
argo             hello-executor-plugin         2      10m

I submit a example plugin workflow to namespace khaos-workflow.

# argo list -n khaos-workflow
NAME          STATUS    AGE   DURATION   PRIORITY   MESSAGE
hello-fzpbc   Running   5m    5m         0
wang@ONINOWANG-MB0 demo % argo get hello-fzpbc -n khaos-workflow
Name:                hello-fzpbc
Namespace:           khaos-workflow
ServiceAccount:      unset (will run with the default ServiceAccount)
Status:              Running
Created:             Tue Apr 02 11:42:03 +0800 (6 minutes ago)
Started:             Tue Apr 02 11:42:03 +0800 (6 minutes ago)
Duration:            6 minutes 2 seconds
Progress:            0/1

STEP            TEMPLATE  PODNAME  DURATION  MESSAGE
 ◷ hello-fzpbc  main

There is only one sidecar container khaos-steps loaded in agent pod, the hello executor plugin was not loaded.

Containers:
  khaos-steps:
    Container ID:
    Image:         ccr.ccs.tencentyun.com/cdb.khaos.eros/khaos-steps-executor:develop
    Image ID:
    Port:          8888/TCP
    Host Port:     0/TCP
    Args:
      server
    State:          Waiting
      Reason:       PodInitializing
    Ready:          False
    Restart Count:  0
    Limits:
      cpu:     200m
      memory:  1Gi
    Requests:
      cpu:        100m
      memory:     512Mi
    Environment:  <none>
    Mounts:
      /var/run/argo from var-run-argo (ro,path="khaos-steps")
      /var/run/secrets/kubernetes.io/serviceaccount from kube-api-access-2pwsk (ro)
  main:
    Container ID:
    Image:         quay.io/argoproj/argoexec:latest
    Image ID:
    Port:          <none>
    Host Port:     <none>
    Command:
      argoexec
    Args:
      agent
      main
    State:          Waiting
      Reason:       PodInitializing
    Ready:          False
    Restart Count:  0
    Limits:
      cpu:     100m
      memory:  256M
    Requests:
      cpu:     10m
      memory:  64M
    Environment:
      ARGO_WORKFLOW_NAME:     hello-fzpbc
      ARGO_WORKFLOW_UID:      ac740ae4-2d67-4fa5-9fa5-246ea6ff935c
      ARGO_AGENT_PATCH_RATE:  1s
      ARGO_PLUGIN_ADDRESSES:  ["http://localhost:8888"]
      ARGO_PLUGIN_NAMES:      ["khaos-steps"]
    Mounts:
      /var/run/argo from var-run-argo (rw)
      /var/run/secrets/kubernetes.io/serviceaccount from kube-api-access-wwhwr (ro)
Conditions:
  Type              Status
  Initialized       False
  Ready             False
  ContainersReady   False
  PodScheduled      True

@shuangkun shuangkun self-assigned this Apr 3, 2024
@shuangkun
Copy link
Member

shuangkun commented Apr 3, 2024

OK, thanks.

@agilgur5 agilgur5 self-assigned this Apr 8, 2024
@agilgur5 agilgur5 added the area/agent Argo Agent that runs for HTTP and Plugin templates label Apr 26, 2024
@agilgur5 agilgur5 changed the title fix: Only get executor plugins in workflow namespace. Fixes #12708 fix!: Only get executor plugins in workflow namespace. Fixes #12708 May 11, 2024
@jswxstw
Copy link
Member Author

jswxstw commented Sep 9, 2024

@agilgur5 Can this PR be merged?

@Joibel
Copy link
Member

Joibel commented Oct 29, 2024

As this stands it's a feature removal - the current behaviour of loading from the controller namespace is documented here so would need amending in this PR.

@jswxstw
Copy link
Member Author

jswxstw commented Oct 31, 2024

As this stands it's a feature removal - the current behaviour of loading from the controller namespace is documented here so would need amending in this PR.

@Joibel Thank you for the reminder!
I have made the modifications, but I'm not sure if I should explicitly indicate the changes here. If I need to indicate them, how should I describe it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/agent Argo Agent that runs for HTTP and Plugin templates area/executor area/plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AgentPod misses serviceaccounts of plugin in controller namespace
4 participants