Skip to content

Commit

Permalink
feat: Remove the PNS executor. Fixes #7804 (#8296)
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Collins <alex_collins@intuit.com>
  • Loading branch information
alexec authored Apr 4, 2022
1 parent 0a36a85 commit 62e0a8c
Show file tree
Hide file tree
Showing 38 changed files with 31 additions and 1,096 deletions.
14 changes: 2 additions & 12 deletions .github/workflows/ci-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,28 +88,18 @@ jobs:
matrix:
include:
- test: test-plugins
containerRuntimeExecutor: emissary
profile: plugins
- test: test-functional
containerRuntimeExecutor: emissary
profile: minimal
- test: test-api
containerRuntimeExecutor: emissary
profile: mysql
- test: test-cli
containerRuntimeExecutor: emissary
profile: mysql
- test: test-cron
containerRuntimeExecutor: emissary
profile: minimal
- test: test-examples
containerRuntimeExecutor: emissary
profile: minimal
- test: test-executor
containerRuntimeExecutor: emissary
profile: minimal
- test: test-executor
containerRuntimeExecutor: pns
profile: minimal
steps:
- uses: actions/checkout@v3
Expand Down Expand Up @@ -144,11 +134,11 @@ jobs:
echo '127.0.0.1 minio' | sudo tee -a /etc/hosts
echo '127.0.0.1 postgres' | sudo tee -a /etc/hosts
echo '127.0.0.1 mysql' | sudo tee -a /etc/hosts
- run: make install PROFILE=${{matrix.profile}} E2E_EXECUTOR=${{matrix.containerRuntimeExecutor}} STATIC_FILES=false
- run: make install PROFILE=${{matrix.profile}} STATIC_FILES=false
- run: make controller $(go env GOPATH)/bin/goreman STATIC_FILES=false
- run: make cli STATIC_FILES=false
if: ${{matrix.test == 'test-api' || matrix.test == 'test-cli'}}
- run: make start PROFILE=${{matrix.profile}} E2E_EXECUTOR=${{matrix.containerRuntimeExecutor}} AUTH_MODE=client STATIC_FILES=false LOG_LEVEL=info API=${{matrix.test == 'test-api' || matrix.test == 'test-cli'}} UI=false > /tmp/argo.log 2>&1 &
- run: make start PROFILE=${{matrix.profile}} AUTH_MODE=client STATIC_FILES=false LOG_LEVEL=info API=${{matrix.test == 'test-api' || matrix.test == 'test-cli'}} UI=false > /tmp/argo.log 2>&1 &
- run: make wait
timeout-minutes: 4
- run: make ${{matrix.test}} E2E_TIMEOUT=1m STATIC_FILES=false
Expand Down
3 changes: 1 addition & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ ARG DOCKER_CHANNEL
ARG DOCKER_VERSION
ARG KUBECTL_VERSION

RUN apk --no-cache add curl procps git tar libcap jq
RUN apk --no-cache add curl git tar jq

COPY hack/arch.sh hack/os.sh /bin/

Expand Down Expand Up @@ -87,7 +87,6 @@ RUN cat .dockerignore >> .gitignore
RUN git status --porcelain | cut -c4- | xargs git update-index --skip-worktree

RUN --mount=type=cache,target=/root/.cache/go-build make dist/argoexec
RUN setcap CAP_SYS_PTRACE,CAP_SYS_CHROOT+ei dist/argoexec

####################################################################################################

Expand Down
8 changes: 1 addition & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ endif
ARGOEXEC_PKGS := $(shell echo cmd/argoexec && go list -f '{{ join .Deps "\n" }}' ./cmd/argoexec/ | grep 'argoproj/argo-workflows/v3/' | cut -c 39-)
CLI_PKGS := $(shell echo cmd/argo && go list -f '{{ join .Deps "\n" }}' ./cmd/argo/ | grep 'argoproj/argo-workflows/v3/' | cut -c 39-)
CONTROLLER_PKGS := $(shell echo cmd/workflow-controller && go list -f '{{ join .Deps "\n" }}' ./cmd/workflow-controller/ | grep 'argoproj/argo-workflows/v3/' | cut -c 39-)
E2E_EXECUTOR ?= emissary
TYPES := $(shell find pkg/apis/workflow/v1alpha1 -type f -name '*.go' -not -name openapi_generated.go -not -name '*generated*' -not -name '*test.go')
CRDS := $(shell find manifests/base/crds -type f -name 'argoproj.io_*.yaml')
SWAGGER_FILES := pkg/apiclient/_.primary.swagger.json \
Expand Down Expand Up @@ -409,13 +408,8 @@ test: server/static/files.go dist/argosay
install: githooks
kubectl get ns $(KUBE_NAMESPACE) || kubectl create ns $(KUBE_NAMESPACE)
kubectl config set-context --current --namespace=$(KUBE_NAMESPACE)
@echo "installing PROFILE=$(PROFILE), E2E_EXECUTOR=$(E2E_EXECUTOR)"
@echo "installing PROFILE=$(PROFILE)"
kubectl kustomize --load-restrictor=LoadRestrictionsNone test/e2e/manifests/$(PROFILE) | sed 's|quay.io/argoproj/|$(IMAGE_NAMESPACE)/|' | sed 's/namespace: argo/namespace: $(KUBE_NAMESPACE)/' | kubectl -n $(KUBE_NAMESPACE) apply --prune -l app.kubernetes.io/part-of=argo -f -
ifneq ($(E2E_EXECUTOR),emissary)
# only change the executor from the default it we need to
kubectl patch cm/workflow-controller-configmap -p "{\"data\": {\"containerRuntimeExecutor\": \"$(E2E_EXECUTOR)\"}}"
kubectl apply -f manifests/quick-start/base/executor/$(E2E_EXECUTOR)
endif
ifeq ($(PROFILE),stress)
kubectl -n $(KUBE_NAMESPACE) apply -f test/stress/massive-workflow.yaml
endif
Expand Down
15 changes: 3 additions & 12 deletions cmd/argoexec/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/argoproj/argo-workflows/v3/workflow/common"
"github.com/argoproj/argo-workflows/v3/workflow/executor"
"github.com/argoproj/argo-workflows/v3/workflow/executor/emissary"
"github.com/argoproj/argo-workflows/v3/workflow/executor/pns"
)

const (
Expand Down Expand Up @@ -76,11 +75,10 @@ func NewRootCommand() *cobra.Command {

func initExecutor() *executor.WorkflowExecutor {
version := argo.GetVersion()
executorType := os.Getenv(common.EnvVarContainerRuntimeExecutor)
log.WithFields(log.Fields{"version": version.Version, "executorType": executorType}).Info("Starting Workflow Executor")
log.WithFields(log.Fields{"version": version.Version}).Info("Starting Workflow Executor")
config, err := clientConfig.ClientConfig()
checkErr(err)
config = restclient.AddUserAgent(config, fmt.Sprintf("argo-workflows/%s argo-executor/%s", version.Version, executorType))
config = restclient.AddUserAgent(config, fmt.Sprintf("argo-workflows/%s argo-executor", version.Version))

logs.AddK8SLogTransportWrapper(config) // lets log all request as we should typically do < 5 per pod, so this is will show up problems

Expand Down Expand Up @@ -108,14 +106,7 @@ func initExecutor() *executor.WorkflowExecutor {
annotationPatchTickDuration, _ := time.ParseDuration(os.Getenv(common.EnvVarProgressPatchTickDuration))
progressFileTickDuration, _ := time.ParseDuration(os.Getenv(common.EnvVarProgressFileTickDuration))

var cre executor.ContainerRuntimeExecutor
log.Infof("Creating a %s executor", executorType)
switch executorType {
case common.ContainerRuntimeExecutorPNS:
cre, err = pns.NewPNSExecutor(clientset, podName, namespace)
default:
cre, err = emissary.New()
}
cre, err := emissary.New()
checkErr(err)

wfExecutor := executor.NewExecutor(
Expand Down
16 changes: 0 additions & 16 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ type Config struct {
// KubeConfig specifies a kube config file for the wait & init containers
KubeConfig *KubeConfig `json:"kubeConfig,omitempty"`

// ContainerRuntimeExecutor specifies the container runtime interface to use, default is emissary
ContainerRuntimeExecutor string `json:"containerRuntimeExecutor,omitempty"`

ContainerRuntimeExecutors ContainerRuntimeExecutors `json:"containerRuntimeExecutors,omitempty"`

// ArtifactRepository contains the default location of an artifact repository for container artifacts
ArtifactRepository wfv1.ArtifactRepository `json:"artifactRepository,omitempty"`

Expand Down Expand Up @@ -110,17 +105,6 @@ type Config struct {
SSO SSOConfig `json:"sso,omitempty"`
}

func (c Config) GetContainerRuntimeExecutor(labels labels.Labels) (string, error) {
name, err := c.ContainerRuntimeExecutors.Select(labels)
if err != nil {
return "", err
}
if name != "" {
return name, nil
}
return c.ContainerRuntimeExecutor, nil
}

func (c Config) GetExecutor() *apiv1.Container {
if c.Executor != nil {
return c.Executor
Expand Down
30 changes: 0 additions & 30 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,9 @@ import (
"testing"

"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
)

func TestDatabaseConfig(t *testing.T) {
assert.Equal(t, "my-host", DatabaseConfig{Host: "my-host"}.GetHostname())
assert.Equal(t, "my-host:1234", DatabaseConfig{Host: "my-host", Port: 1234}.GetHostname())
}

func TestContainerRuntimeExecutor(t *testing.T) {
t.Run("Default", func(t *testing.T) {
c := Config{ContainerRuntimeExecutor: "foo"}
executor, err := c.GetContainerRuntimeExecutor(labels.Set{})
assert.NoError(t, err)
assert.Equal(t, "foo", executor)
})
t.Run("Error", func(t *testing.T) {
c := Config{ContainerRuntimeExecutor: "foo", ContainerRuntimeExecutors: ContainerRuntimeExecutors{
{Name: "bar", Selector: metav1.LabelSelector{
MatchLabels: map[string]string{"!": "!"},
}},
}}
_, err := c.GetContainerRuntimeExecutor(labels.Set{})
assert.Error(t, err)
})
t.Run("NoError", func(t *testing.T) {
c := Config{ContainerRuntimeExecutor: "foo", ContainerRuntimeExecutors: ContainerRuntimeExecutors{
{Name: "bar", Selector: metav1.LabelSelector{
MatchLabels: map[string]string{"baz": "qux"},
}},
}}
executor, err := c.GetContainerRuntimeExecutor(labels.Set(map[string]string{"baz": "qux"}))
assert.NoError(t, err)
assert.Equal(t, "bar", executor)
})
}
10 changes: 1 addition & 9 deletions config/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,9 @@ func Test_parseConfigMap(t *testing.T) {
err := parseConfigMap(&apiv1.ConfigMap{}, c)
assert.NoError(t, err)
})
t.Run("Config", func(t *testing.T) {
c := &Config{}
err := parseConfigMap(&apiv1.ConfigMap{Data: map[string]string{"config": "containerRuntimeExecutor: pns"}}, c)
if assert.NoError(t, err) {
assert.Equal(t, "pns", c.ContainerRuntimeExecutor)
}
})
t.Run("Complex", func(t *testing.T) {
c := &Config{}
err := parseConfigMap(&apiv1.ConfigMap{Data: map[string]string{"containerRuntimeExecutor": "pns", "artifactRepository": ` archiveLogs: true
err := parseConfigMap(&apiv1.ConfigMap{Data: map[string]string{"artifactRepository": ` archiveLogs: true
s3:
bucket: my-bucket
endpoint: minio:9000
Expand All @@ -34,7 +27,6 @@ func Test_parseConfigMap(t *testing.T) {
name: my-minio-cred
key: secretkey`}}, c)
if assert.NoError(t, err) {
assert.Equal(t, "pns", c.ContainerRuntimeExecutor)
assert.NotEmpty(t, c.ArtifactRepository)
}
})
Expand Down
4 changes: 0 additions & 4 deletions docs/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,10 @@ spec:
| Name | Type | Default | Description |
|------|------|---------|-------------|
| `ARGO_CONTAINER_RUNTIME_EXECUTOR` | `string` | `"docker"` | The name of the container runtime executor. |
| `ARGO_KUBELET_PORT` | `int` | `10250` | The port to the Kubelet API. |
| `ARGO_KUBELET_INSECURE` | `bool` | `false` | Whether to disable the TLS verification. |
| `EXECUTOR_RETRY_BACKOFF_DURATION` | `time.Duration` | `1s` | The retry backoff duration when the workflow executor performs retries. |
| `EXECUTOR_RETRY_BACKOFF_FACTOR` | `float` | `1.6` | The retry backoff factor when the workflow executor performs retries. |
| `EXECUTOR_RETRY_BACKOFF_JITTER` | `float` | `0.5` | The retry backoff jitter when the workflow executor performs retries. |
| `EXECUTOR_RETRY_BACKOFF_STEPS` | `int` | `5` | The retry backoff steps when the workflow executor performs retries. |
| `PNS_PRIVILEGED` | `bool` | `false` | Whether to always set privileged on for PNS when PNS executor is used. |
| `REMOVE_LOCAL_ART_PATH` | `bool` | `false` | Whether to remove local artifacts. |
| `RESOURCE_STATE_CHECK_INTERVAL` | `time.Duration` | `5s` | The time interval between resource status checks against the specified success and failure conditions. |
| `WAIT_CONTAINER_STATUS_CHECK_INTERVAL` | `time.Duration` | `5s` | The time interval for wait container to check whether the containers have completed. |
Expand Down
2 changes: 1 addition & 1 deletion docs/running-locally.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ make argoexec-image
4. Run the profile in a terminal window

```shell
make start PROFILE=minimal E2E_EXECUTOR=emissary AUTH_MODE=client STATIC_FILES=false LOG_LEVEL=info API=true UI=false
make start PROFILE=minimal AUTH_MODE=client STATIC_FILES=false LOG_LEVEL=info API=true UI=false
```

5. Run the test in your IDE
Expand Down
4 changes: 4 additions & 0 deletions docs/workflow-controller-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ data:
# Specifies the container runtime interface to use (default: emissary)
# must be one of: docker, kubelet, k8sapi, pns, emissary
# It has lower precedence than either `--container-runtime-executor` and `containerRuntimeExecutors`.
# (removed in v3.4)
containerRuntimeExecutor: emissary

# Specifies the executor to use.
Expand All @@ -158,6 +159,7 @@ data:
#
# The list is in order of precedence; the first matching executor is used.
# This has precedence over `containerRuntimeExecutor`.
# (removed in v3.4)
containerRuntimeExecutors: |
- name: emissary
selector:
Expand All @@ -173,9 +175,11 @@ data:
dockerSockPath: /var/someplace/else/docker.sock

# kubelet port when using kubelet executor (default: 10250) (kubelet executor will be deprecated use emissary instead)
# (removed in v3.4)
kubeletPort: 10250

# disable the TLS verification of the kubelet executor (default: false)
# (removed in v3.4)
kubeletInsecure: false

# The command/args for each image, needed when the command is not specified and the emissary executor is used.
Expand Down
8 changes: 5 additions & 3 deletions docs/workflow-executors.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ The emissary will exit with code 64 if it fails. This may indicate a bug in the

## Docker (docker)

⚠️Deprecated.
⚠️Deprecated. Removed in v3.4.

**default in <= v3.2**

Expand All @@ -67,7 +67,7 @@ The emissary will exit with code 64 if it fails. This may indicate a bug in the

## Kubelet (kubelet)

⚠️Deprecated.
⚠️Deprecated. Removed in v3.4.

* Secure
* No `privileged` access
Expand All @@ -84,7 +84,7 @@ The emissary will exit with code 64 if it fails. This may indicate a bug in the

## Kubernetes API (k8sapi)

⚠️Deprecated.
⚠️Deprecated. Removed in v3.4.

* Reliability:
* Works on GKE Autopilot
Expand All @@ -103,6 +103,8 @@ The emissary will exit with code 64 if it fails. This may indicate a bug in the

## Process Namespace Sharing (pns)

⚠️Deprecated. Removed in v3.4.

* More secure:
* No `privileged` access
* cannot escape the privileges of the pod's service account
Expand Down
4 changes: 1 addition & 3 deletions examples/selected-executor-workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ kind: Workflow
metadata:
generateName: selected-executor-
labels:
# run this workflow as a part of our test suite
workflows.argoproj.io/test: "true"
# use the pns executor, rather than the default (typically emissary)
workflows.argoproj.io/container-runtime-executor: pns
annotations:
Expand All @@ -17,7 +15,7 @@ metadata:
e.g. have a certain labels use certain executors.
# this workflow will only run on workflows version v3.0.0
workflows.argoproj.io/version: ">= 3.0.0"
workflows.argoproj.io/version: ">= 3.0.0 < 3.4.0"
spec:
entrypoint: main
templates:
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ require (
github.com/imkira/go-interpol v1.1.0 // indirect
github.com/klauspost/pgzip v1.2.5
github.com/minio/minio-go/v7 v7.0.23
github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b
github.com/prometheus/client_golang v1.12.1
github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.33.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -913,8 +913,6 @@ github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HK
github.com/mitchellh/go-homedir v1.0.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b h1:9+ke9YJ9KGWw5ANXK6ozjoK47uI3uNbXv4YVINBnGm8=
github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b/go.mod h1:r1VsdOzOPt1ZSrGZWFoNhsAedKnEd6r9Np1+5blZCWk=
github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eIhFbs2+BFgRtFPeD/KE+zxI=
github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
Expand Down
2 changes: 0 additions & 2 deletions hack/test-examples.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ set -eu -o pipefail

# Load the configmaps that contains the parameter values used for certain examples.
kubectl apply -f examples/configmaps/simple-parameters-configmap.yaml
# Needed for examples/selected-executor-workflow.yaml.
kubectl apply -f manifests/quick-start/base/executor/pns/executor-role.yaml

echo "Checking for banned images..."
grep -lR 'workflows.argoproj.io/test' examples/* | while read f ; do
Expand Down
9 changes: 0 additions & 9 deletions manifests/quick-start-minimal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1164,15 +1164,6 @@ data:
secretKeySecret:
name: my-minio-cred
key: secretkey
containerRuntimeExecutors: |
- name: emissary
selector:
matchLabels:
workflows.argoproj.io/container-runtime-executor: emissary
- name: pns
selector:
matchLabels:
workflows.argoproj.io/container-runtime-executor: pns
executor: |
resources:
requests:
Expand Down
9 changes: 0 additions & 9 deletions manifests/quick-start-mysql.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1164,15 +1164,6 @@ data:
secretKeySecret:
name: my-minio-cred
key: secretkey
containerRuntimeExecutors: |
- name: emissary
selector:
matchLabels:
workflows.argoproj.io/container-runtime-executor: emissary
- name: pns
selector:
matchLabels:
workflows.argoproj.io/container-runtime-executor: pns
executor: |
resources:
requests:
Expand Down
9 changes: 0 additions & 9 deletions manifests/quick-start-postgres.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1164,15 +1164,6 @@ data:
secretKeySecret:
name: my-minio-cred
key: secretkey
containerRuntimeExecutors: |
- name: emissary
selector:
matchLabels:
workflows.argoproj.io/container-runtime-executor: emissary
- name: pns
selector:
matchLabels:
workflows.argoproj.io/container-runtime-executor: pns
executor: |
resources:
requests:
Expand Down
Loading

0 comments on commit 62e0a8c

Please sign in to comment.