Skip to content

Commit

Permalink
fix: removed deprecated k8sapi executor. Fixes #7802 (#8205)
Browse files Browse the repository at this point in the history
Signed-off-by: Rohan Kumar <[email protected]>
  • Loading branch information
rohankmr414 authored Mar 22, 2022
1 parent 4d50798 commit 8a1fbb8
Show file tree
Hide file tree
Showing 11 changed files with 9 additions and 47 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/ci-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ jobs:
- test: test-executor
containerRuntimeExecutor: docker
profile: minimal
- test: test-executor
containerRuntimeExecutor: k8sapi
profile: minimal
- test: test-executor
containerRuntimeExecutor: kubelet
profile: minimal
Expand Down
3 changes: 0 additions & 3 deletions cmd/argoexec/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/argoproj/argo-workflows/v3/workflow/executor"
"github.com/argoproj/argo-workflows/v3/workflow/executor/docker"
"github.com/argoproj/argo-workflows/v3/workflow/executor/emissary"
"github.com/argoproj/argo-workflows/v3/workflow/executor/k8sapi"
"github.com/argoproj/argo-workflows/v3/workflow/executor/kubelet"
"github.com/argoproj/argo-workflows/v3/workflow/executor/pns"
)
Expand Down Expand Up @@ -114,8 +113,6 @@ func initExecutor() *executor.WorkflowExecutor {
var cre executor.ContainerRuntimeExecutor
log.Infof("Creating a %s executor", executorType)
switch executorType {
case common.ContainerRuntimeExecutorK8sAPI:
cre = k8sapi.NewK8sAPIExecutor(clientset, config, podName, namespace)
case common.ContainerRuntimeExecutorKubelet:
cre, err = kubelet.NewKubeletExecutor(namespace, podName)
case common.ContainerRuntimeExecutorPNS:
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/argo_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ func (s *ArgoServerSuite) TestWorkflowService() {
})

s.Run("Terminate", func() {
s.Need(fixtures.None(fixtures.K8SAPI, fixtures.Kubelet))
s.Need(fixtures.None(fixtures.Kubelet))
s.e().PUT("/api/v1/workflows/argo/" + name + "/terminate").
Expect().
Status(200)
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/fixtures/needs.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ var (
return os.Getenv("CI") != "", "CI"
}
BaseLayerArtifacts Need = func(s *E2ESuite) (bool, string) {
met, _ := None(K8SAPI, Kubelet)(s)
met, _ := None(Kubelet)(s)
return met, "base layer artifact support"
}
Docker = Executor("docker")
Emissary = Executor("emissary")
K8SAPI = Executor("k8sapi")
Kubelet = Executor("kubelet")
PNS = Executor("pns")
)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/run_as_not_root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (s *RunAsNonRootSuite) TestRunAsNonRootWorkflow() {
}

func (s *RunAsNonRootSuite) TestRunAsNonRootWithOutputParams() {
s.Need(fixtures.None(fixtures.Docker, fixtures.K8SAPI, fixtures.Kubelet))
s.Need(fixtures.None(fixtures.Docker, fixtures.Kubelet))
s.Given().
Workflow("@smoke/runasnonroot-output-params-pipeline.yaml").
When().
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/signals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type SignalsSuite struct {
func (s *SignalsSuite) SetupSuite() {
s.E2ESuite.SetupSuite()
// Because k8ssapi and kubelet execute `sh -c 'kill 15 1'` to they do not work.
s.Need(fixtures.None(fixtures.K8SAPI, fixtures.Kubelet))
s.Need(fixtures.None(fixtures.Kubelet))
}

func (s *SignalsSuite) TestStopBehavior() {
Expand Down
3 changes: 0 additions & 3 deletions workflow/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,6 @@ const (
// ContainerRuntimeExecutorKubelet to use the kubelet as container runtime executor
ContainerRuntimeExecutorKubelet = "kubelet"

// ContainerRuntimeExecutorK8sAPI to use the Kubernetes API server as container runtime executor
ContainerRuntimeExecutorK8sAPI = "k8sapi"

// ContainerRuntimeExecutorPNS indicates to use process namespace sharing as the container runtime executor
ContainerRuntimeExecutorPNS = "pns"

Expand Down
24 changes: 0 additions & 24 deletions workflow/controller/workflowpod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -764,30 +764,6 @@ func TestVolumeAndVolumeMounts(t *testing.T) {
assert.Equal(t, "var-run-argo", pod.Spec.Containers[1].VolumeMounts[1].Name)
})

// For K8sAPI executor
t.Run("K8SAPI", func(t *testing.T) {
ctx := context.Background()
woc := newWoc()
woc.volumes = volumes
woc.execWf.Spec.Templates[0].Container.VolumeMounts = volumeMounts
woc.controller.Config.ContainerRuntimeExecutor = common.ContainerRuntimeExecutorK8sAPI

tmplCtx, err := woc.createTemplateContext(wfv1.ResourceScopeLocal, "")
assert.NoError(t, err)
_, err = woc.executeContainer(ctx, woc.execWf.Spec.Entrypoint, tmplCtx.GetTemplateScope(), &woc.execWf.Spec.Templates[0], &wfv1.WorkflowStep{}, &executeTemplateOpts{})
assert.NoError(t, err)
pods, err := listPods(woc)
assert.NoError(t, err)
assert.Len(t, pods.Items, 1)
pod := pods.Items[0]
assert.Equal(t, 2, len(pod.Spec.Volumes))
assert.Equal(t, "var-run-argo", pod.Spec.Volumes[0].Name)
assert.Equal(t, "volume-name", pod.Spec.Volumes[1].Name)
assert.Equal(t, 2, len(pod.Spec.Containers[1].VolumeMounts))
assert.Equal(t, "volume-name", pod.Spec.Containers[1].VolumeMounts[0].Name)
assert.Equal(t, "var-run-argo", pod.Spec.Containers[1].VolumeMounts[1].Name)
})

// For emissary executor
t.Run("Emissary", func(t *testing.T) {
ctx := context.Background()
Expand Down
4 changes: 2 additions & 2 deletions workflow/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,8 +508,8 @@ func (we *WorkflowExecutor) SaveParameters(ctx context.Context) error {
var output *wfv1.AnyString
if we.isBaseImagePath(param.ValueFrom.Path) {
executorType := os.Getenv(common.EnvVarContainerRuntimeExecutor)
if executorType == common.ContainerRuntimeExecutorK8sAPI || executorType == common.ContainerRuntimeExecutorKubelet {
log.Infof("Copying output parameter %s from base image layer %s is not supported for k8sapi and kubelet executors. "+
if executorType == common.ContainerRuntimeExecutorKubelet {
log.Infof("Copying output parameter %s from base image layer %s is not supported for kubelet executor. "+
"Consider using an emptyDir volume: https://argoproj.github.io/argo-workflows/empty-dir/.", param.Name, param.ValueFrom.Path)
continue
}
Expand Down
2 changes: 1 addition & 1 deletion workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ func (ctx *templateValidationCtx) validateBaseImageOutputs(tmpl *wfv1.Template)
}
}
}
case common.ContainerRuntimeExecutorK8sAPI, common.ContainerRuntimeExecutorKubelet:
case common.ContainerRuntimeExecutorKubelet:
// for kubelet/k8s fail validation if we detect artifact is copied from base image layer
errMsg := fmt.Sprintf("%s executor does not support outputs from base image layer. Use an emptyDir: https://argoproj.github.io/argo-workflows/empty-dir/", ctx.ContainerRuntimeExecutor)
for _, out := range tmpl.Outputs.Artifacts {
Expand Down
8 changes: 2 additions & 6 deletions workflow/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1626,9 +1626,9 @@ func TestBaseImageOutputVerify(t *testing.T) {
wfNonPathOutputParam := unmarshalWf(nonPathOutputParameter)
var err error

for _, executor := range []string{common.ContainerRuntimeExecutorK8sAPI, common.ContainerRuntimeExecutorKubelet, common.ContainerRuntimeExecutorPNS, common.ContainerRuntimeExecutorDocker, common.ContainerRuntimeExecutorDocker, common.ContainerRuntimeExecutorEmissary, ""} {
for _, executor := range []string{common.ContainerRuntimeExecutorKubelet, common.ContainerRuntimeExecutorPNS, common.ContainerRuntimeExecutorDocker, common.ContainerRuntimeExecutorDocker, common.ContainerRuntimeExecutorEmissary, ""} {
switch executor {
case common.ContainerRuntimeExecutorK8sAPI, common.ContainerRuntimeExecutorKubelet:
case common.ContainerRuntimeExecutorKubelet:
_, err = ValidateWorkflow(wftmplGetter, cwftmplGetter, wfBaseOutArt, ValidateOpts{ContainerRuntimeExecutor: executor})
assert.Error(t, err)
_, err = ValidateWorkflow(wftmplGetter, cwftmplGetter, wfBaseOutParam, ValidateOpts{ContainerRuntimeExecutor: executor})
Expand Down Expand Up @@ -2524,10 +2524,6 @@ func TestDagAndStepLevelOutputArtifactsForDiffExecutor(t *testing.T) {
_, err := validateWithOptions(dagAndStepLevelOutputArtifacts, ValidateOpts{ContainerRuntimeExecutor: common.ContainerRuntimeExecutorPNS})
assert.NoError(t, err)
})
t.Run("K8SExecutor", func(t *testing.T) {
_, err := validateWithOptions(dagAndStepLevelOutputArtifacts, ValidateOpts{ContainerRuntimeExecutor: common.ContainerRuntimeExecutorK8sAPI})
assert.NoError(t, err)
})
t.Run("KubeletExecutor", func(t *testing.T) {
_, err := validateWithOptions(dagAndStepLevelOutputArtifacts, ValidateOpts{ContainerRuntimeExecutor: common.ContainerRuntimeExecutorKubelet})
assert.NoError(t, err)
Expand Down

0 comments on commit 8a1fbb8

Please sign in to comment.