Skip to content

Commit

Permalink
fix(executor): Fix compatibility issue when selfLink is no longer pop…
Browse files Browse the repository at this point in the history
…ulated for k8s>=1.21. Fixes #6045 (#6014)
  • Loading branch information
terrytangyuan committed Jun 2, 2021
1 parent 1f3493a commit 803855b
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 2 deletions.
39 changes: 39 additions & 0 deletions test/e2e/resource_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,45 @@ spec:
})
}

func (s *ResourceTemplateSuite) TestResourceTemplateWithPod() {
s.Given().
Workflow(`
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: k8s-resource-tmpl-with-pod-
spec:
serviceAccount: argo
entrypoint: main
templates:
- name: main
serviceAccountName: argo
resource:
action: create
successCondition: status.phase == Succeeded
failureCondition: status.phase == Failed
manifest: |
apiVersion: v1
kind: Pod
metadata:
generateName: k8s-pod-resource-
spec:
serviceAccountName: argo
containers:
- name: argosay-container
image: argoproj/argosay:v2
command: ["/argosay"]
restartPolicy: Never
`).
When().
SubmitWorkflow().
WaitForWorkflow().
Then().
ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.WorkflowSucceeded, status.Phase)
})
}

func TestResourceTemplateSuite(t *testing.T) {
suite.Run(t, new(ResourceTemplateSuite))
}
21 changes: 19 additions & 2 deletions workflow/executor/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
log "github.com/sirupsen/logrus"
"github.com/tidwall/gjson"
apierr "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -59,12 +60,28 @@ func (we *WorkflowExecutor) ExecResource(action string, manifestPath string, fla
if resourceName == "" || resourceKind == "" {
return "", "", "", errors.New(errors.CodeBadRequest, "Kind and name are both required but at least one of them is missing from the manifest")
}
resourceFullName := fmt.Sprintf("%s.%s/%s", resourceKind, resourceGroup, resourceName)
selfLink := obj.GetSelfLink()
resourceFullName := fmt.Sprintf("%s.%s/%s", strings.ToLower(resourceKind), resourceGroup, resourceName)
selfLink := inferObjectSelfLink(obj)
log.Infof("Resource: %s/%s. SelfLink: %s", obj.GetNamespace(), resourceFullName, selfLink)
return obj.GetNamespace(), resourceFullName, selfLink, nil
}

func inferObjectSelfLink(obj unstructured.Unstructured) string {
gvk := obj.GroupVersionKind()
// This is the best guess we can do here and is what `kubectl` uses under the hood. Hopefully future versions of the
// REST client would remove the need to infer the plural name.
pluralGVR, _ := meta.UnsafeGuessKindToResource(gvk)
var selfLinkPrefix string
if gvk.Group == "" {
selfLinkPrefix = "api"
} else {
selfLinkPrefix = "apis"
}
// We cannot use `obj.GetSelfLink()` directly since it is deprecated and will be removed after Kubernetes 1.21: https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1164-remove-selflink
return fmt.Sprintf("%s/%s/namespaces/%s/%s/%s",
selfLinkPrefix, obj.GetAPIVersion(), obj.GetNamespace(), pluralGVR.Resource, obj.GetName())
}

func (we *WorkflowExecutor) getKubectlArguments(action string, manifestPath string, flags []string) ([]string, error) {
buff, err := ioutil.ReadFile(manifestPath)
if err != nil {
Expand Down
29 changes: 29 additions & 0 deletions workflow/executor/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/kubernetes/fake"

wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
Expand Down Expand Up @@ -150,3 +152,30 @@ func TestResourceConditionsMatching(t *testing.T) {
assert.Error(t, err, "Neither success condition nor the failure condition has been matched. Retrying...")
assert.True(t, finished)
}

// TestInferSelfLink tests whether the inferred self link for k8s objects are correct.
func TestInferSelfLink(t *testing.T) {
obj := unstructured.Unstructured{}
obj.SetNamespace("test-namespace")
obj.SetName("test-name")
obj.SetGroupVersionKind(schema.GroupVersionKind{
Group: "",
Version: "v1",
Kind: "Pod",
})
assert.Equal(t, "api/v1/namespaces/test-namespace/pods/test-name", inferObjectSelfLink(obj))

obj.SetGroupVersionKind(schema.GroupVersionKind{
Group: "test.group",
Version: "v1",
Kind: "TestKind",
})
assert.Equal(t, "apis/test.group/v1/namespaces/test-namespace/testkinds/test-name", inferObjectSelfLink(obj))

obj.SetGroupVersionKind(schema.GroupVersionKind{
Group: "test.group",
Version: "v1",
Kind: "Duty",
})
assert.Equal(t, "apis/test.group/v1/namespaces/test-namespace/duties/test-name", inferObjectSelfLink(obj))
}

0 comments on commit 803855b

Please sign in to comment.