Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

Commit

Permalink
Merge pull request #267 from wonderflow/fixig
Browse files Browse the repository at this point in the history
don't block app from running if no definition found
  • Loading branch information
hongchaodeng authored Oct 26, 2020
2 parents 81eeb8d + 964c9df commit d992f48
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 8 deletions.
3 changes: 3 additions & 0 deletions apis/core/v1alpha2/core_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ type WorkloadTrait struct {

// Reference to a trait created by an ApplicationConfiguration.
Reference runtimev1alpha1.TypedReference `json:"traitRef"`

// Message will allow controller to leave some additional information for this trait
Message string `json:"message,omitempty"`
}

// A ScopeStatus represents the state of a scope.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,10 @@ spec:
description: A WorkloadTrait represents a trait associated
with a workload and its status
properties:
message:
description: Message will allow controller to leave some
additional information for this trait
type: string
status:
description: Status is a place holder for a customized
controller to fill if it needs a single place to summarize
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package applicationconfiguration

import (
"context"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2"
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util"
)

var _ = Describe("CRD without definition can run in an ApplicationConfiguration", func() {
ctx := context.Background()
It("run workload and trait without CRD", func() {

By("Creating CRD foo.crdtest1.com")
// Create a crd for appconfig dependency test
crd = crdv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "foo.crdtest1.com",
Labels: map[string]string{"crd": "dependency"},
},
Spec: crdv1.CustomResourceDefinitionSpec{
Group: "crdtest1.com",
Names: crdv1.CustomResourceDefinitionNames{
Kind: "Foo",
ListKind: "FooList",
Plural: "foo",
Singular: "foo",
},
Versions: []crdv1.CustomResourceDefinitionVersion{{
Name: "v1",
Served: true,
Storage: true,
Schema: &crdv1.CustomResourceValidation{
OpenAPIV3Schema: &crdv1.JSONSchemaProps{
Type: "object",
Properties: map[string]crdv1.JSONSchemaProps{
"spec": {
Type: "object",
Properties: map[string]crdv1.JSONSchemaProps{
"key": {Type: "string"},
}}}}}},
},
Scope: crdv1.NamespaceScoped,
},
}
Expect(k8sClient.Create(context.Background(), &crd)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))

By("Creating namespace trait-no-def-test")
namespace := "trait-no-def-test"
var ns = corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}
Expect(k8sClient.Create(ctx, &ns)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))

By("creating component using workload by foo.crdtest1.com without definition")
tempFoo := &unstructured.Unstructured{}
tempFoo.SetAPIVersion("crdtest1.com/v1")
tempFoo.SetKind("Foo")
tempFoo.SetNamespace(namespace)
// Define a workload
wl := tempFoo.DeepCopy()
// Set Name so we can get easily
wlname := "test-workload"
wl.SetName(wlname)
// Create a component
componentName := "component"
comp := v1alpha2.Component{
ObjectMeta: metav1.ObjectMeta{
Name: componentName,
Namespace: namespace,
},
Spec: v1alpha2.ComponentSpec{
Workload: runtime.RawExtension{
Object: wl,
},
},
}
Expect(k8sClient.Create(ctx, &comp)).Should(BeNil())

By("Create application configuration with trait using foo.crdtest1.com without definition")
tr := tempFoo.DeepCopy()
// Set Name so we can get easily
trname := "test-trait"
tr.SetName(trname)
appConfigName := "appconfig-trait-no-def"
appConfig := v1alpha2.ApplicationConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: appConfigName,
Namespace: namespace,
},
Spec: v1alpha2.ApplicationConfigurationSpec{
Components: []v1alpha2.ApplicationConfigurationComponent{{
ComponentName: componentName,
Traits: []v1alpha2.ComponentTrait{{
Trait: runtime.RawExtension{
Object: tr,
}}},
}}},
}
By("Creating application config")
Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil())

By("Reconcile")
appconfigKey := client.ObjectKey{
Name: appConfigName,
Namespace: namespace,
}
req := reconcile.Request{NamespacedName: appconfigKey}
Expect(func() error { _, err := reconciler.Reconcile(req); return err }()).Should(BeNil())

By("Checking that workload should be created")
workloadKey := client.ObjectKey{
Name: wlname,
Namespace: namespace,
}
workloadFoo := tempFoo.DeepCopy()
Eventually(func() error {
err := k8sClient.Get(ctx, workloadKey, workloadFoo)
if err != nil {
// Try 3 (= 1s/300ms) times
reconciler.Reconcile(req)
}
return err
}, time.Second, 300*time.Millisecond).Should(BeNil())

By("Checking that trait should be created")
traitKey := client.ObjectKey{
Name: trname,
Namespace: namespace,
}
traitFoo := tempFoo.DeepCopy()
Eventually(func() error {
err := k8sClient.Get(ctx, traitKey, traitFoo)
if err != nil {
// Try 3 (= 1s/300ms) times
reconciler.Reconcile(req)
}
return err
}, time.Second, 300*time.Millisecond).Should(BeNil())

By("Checking the application status has right warning message")
Expect(func() string {
err := k8sClient.Get(ctx, appconfigKey, &appConfig)
if err != nil {
return ""
}
return appConfig.Status.Workloads[0].Traits[0].Message
}()).Should(Equal(util.DummyTraitMessage))
})

})
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/crossplane/oam-kubernetes-runtime/pkg/controller"
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam"
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper"
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util"
)

const (
Expand Down Expand Up @@ -437,6 +438,9 @@ type Trait struct {

// HasDep indicates whether this resource has dependencies and unready to be applied.
HasDep bool

// Definition indicates the trait's definition
Definition v1alpha2.TraitDefinition
}

// Status produces the status of this workload and its traits, suitable for use
Expand All @@ -453,7 +457,10 @@ func (w Workload) Status() v1alpha2.WorkloadStatus {
Traits: make([]v1alpha2.WorkloadTrait, len(w.Traits)),
Scopes: make([]v1alpha2.WorkloadScope, len(w.Scopes)),
}
for i := range w.Traits {
for i, tr := range w.Traits {
if tr.Definition.Name == util.Dummy && tr.Definition.Spec.Reference.Name == util.Dummy {
acw.Traits[i].Message = util.DummyTraitMessage
}
acw.Traits[i].Reference = runtimev1alpha1.TypedReference{
APIVersion: w.Traits[i].Object.GetAPIVersion(),
Kind: w.Traits[i].Object.GetKind(),
Expand Down
6 changes: 5 additions & 1 deletion pkg/controller/v1alpha2/applicationconfiguration/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -163,7 +164,7 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati

// pass through labels and annotation from app-config to trait
util.PassLabelAndAnnotation(ac, t)
traits = append(traits, &Trait{Object: *t})
traits = append(traits, &Trait{Object: *t, Definition: *traitDef})
traitDefs = append(traitDefs, *traitDef)
}
if err := SetWorkloadInstanceName(traitDefs, w, c); err != nil {
Expand Down Expand Up @@ -215,6 +216,9 @@ func (r *components) renderTrait(ctx context.Context, ct v1alpha2.ComponentTrait

traitDef, err := util.FetchTraitDefinition(ctx, r.client, r.dm, t)
if err != nil {
if apierrors.IsNotFound(err) {
return t, util.GetDummyTraitDefinition(t), nil
}
return nil, nil, errors.Wrapf(err, errFmtGetTraitDefinition, t.GetAPIVersion(), t.GetKind(), t.GetName())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"encoding/json"
"testing"

"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock"

"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
Expand All @@ -43,6 +41,7 @@ import (
"github.com/crossplane/oam-kubernetes-runtime/apis/core"
"github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2"
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam"
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock"
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util"
)

Expand Down Expand Up @@ -392,7 +391,8 @@ func TestRenderComponents(t *testing.T) {
oam.LabelAppComponentRevision: revisionName2,
oam.LabelOAMResourceType: oam.ResourceTypeTrait,
})
return &Trait{Object: *t}
return &Trait{Object: *t,
Definition: v1alpha2.TraitDefinition{ObjectMeta: metav1.ObjectMeta{Name: "coolTrait"}, Spec: v1alpha2.TraitDefinitionSpec{RevisionEnabled: true}}}
}(),
},
RevisionEnabled: true,
Expand Down Expand Up @@ -489,7 +489,7 @@ func TestRenderComponents(t *testing.T) {
if err := fieldpath.Pave(tr.Object).SetValue("spec.workload.path", workloadRef); err != nil {
t.Fail()
}
return &Trait{Object: *tr}
return &Trait{Object: *tr, Definition: v1alpha2.TraitDefinition{Spec: v1alpha2.TraitDefinitionSpec{WorkloadRefPath: "spec.workload.path"}}}
}(),
},
Scopes: []unstructured.Unstructured{},
Expand Down
40 changes: 40 additions & 0 deletions pkg/oam/util/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -39,6 +41,12 @@ var (
const (
// TraitPrefixKey is prefix of trait name
TraitPrefixKey = "trait"

// Dummy used for dummy definition
Dummy = "dummy"

// DummyTraitMessage is a message for trait which don't have definition found
DummyTraitMessage = "No valid TraitDefinition found, all framework capabilities will work as default or disabled"
)

const (
Expand Down Expand Up @@ -111,6 +119,34 @@ func FetchWorkload(ctx context.Context, c client.Client, mLog logr.Logger, oamTr
return &workload, nil
}

// GetDummyTraitDefinition will generate a dummy TraitDefinition for CustomResource that won't block app from running.
// OAM runtime will report warning if they got this dummy definition.
func GetDummyTraitDefinition(u *unstructured.Unstructured) *v1alpha2.TraitDefinition {
return &v1alpha2.TraitDefinition{
TypeMeta: metav1.TypeMeta{Kind: v1alpha2.TraitDefinitionKind, APIVersion: v1alpha2.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{Name: Dummy, Annotations: map[string]string{
"apiVersion": u.GetAPIVersion(),
"kind": u.GetKind(),
"name": u.GetName(),
}},
Spec: v1alpha2.TraitDefinitionSpec{Reference: v1alpha2.DefinitionReference{Name: Dummy}},
}
}

// GetDummyWorkloadDefinition will generate a dummy WorkloadDefinition for CustomResource that won't block app from running.
// OAM runtime will report warning if they got this dummy definition.
func GetDummyWorkloadDefinition(u *unstructured.Unstructured) *v1alpha2.WorkloadDefinition {
return &v1alpha2.WorkloadDefinition{
TypeMeta: metav1.TypeMeta{Kind: v1alpha2.WorkloadDefinitionKind, APIVersion: v1alpha2.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{Name: Dummy, Annotations: map[string]string{
"apiVersion": u.GetAPIVersion(),
"kind": u.GetKind(),
"name": u.GetName(),
}},
Spec: v1alpha2.WorkloadDefinitionSpec{Reference: v1alpha2.DefinitionReference{Name: Dummy}},
}
}

// FetchScopeDefinition fetch corresponding scopeDefinition given a scope
func FetchScopeDefinition(ctx context.Context, r client.Reader, dm discoverymapper.DiscoveryMapper,
scope *unstructured.Unstructured) (*v1alpha2.ScopeDefinition, error) {
Expand Down Expand Up @@ -172,6 +208,10 @@ func FetchWorkloadChildResources(ctx context.Context, mLog logr.Logger, r client
// Fetch the corresponding workloadDefinition CR
workloadDefinition, err := FetchWorkloadDefinition(ctx, r, dm, workload)
if err != nil {
// No definition will won't block app from running
if apierrors.IsNotFound(err) {
return nil, nil
}
return nil, err
}
return fetchChildResources(ctx, mLog, r, workload, workloadDefinition.Spec.ChildResourceKinds)
Expand Down
25 changes: 25 additions & 0 deletions pkg/oam/util/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1182,3 +1182,28 @@ func TestAddLabels(t *testing.T) {
assert.Equal(t, wantObj.GetLabels(), obj.GetLabels())
}
}

func TestGetDummy(t *testing.T) {
var u = &unstructured.Unstructured{}
u.SetKind("Testkind")
u.SetAPIVersion("test.api/v1")
u.SetName("testdummy")
assert.Equal(t, &v1alpha2.TraitDefinition{
TypeMeta: metav1.TypeMeta{Kind: v1alpha2.TraitDefinitionKind, APIVersion: v1alpha2.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{Name: "dummy", Annotations: map[string]string{
"apiVersion": u.GetAPIVersion(),
"kind": u.GetKind(),
"name": u.GetName(),
}},
Spec: v1alpha2.TraitDefinitionSpec{Reference: v1alpha2.DefinitionReference{Name: "dummy"}},
}, util.GetDummyTraitDefinition(u))
assert.Equal(t, &v1alpha2.WorkloadDefinition{
TypeMeta: metav1.TypeMeta{Kind: v1alpha2.WorkloadDefinitionKind, APIVersion: v1alpha2.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{Name: "dummy", Annotations: map[string]string{
"apiVersion": u.GetAPIVersion(),
"kind": u.GetKind(),
"name": u.GetName(),
}},
Spec: v1alpha2.WorkloadDefinitionSpec{Reference: v1alpha2.DefinitionReference{Name: "dummy"}},
}, util.GetDummyWorkloadDefinition(u))
}
3 changes: 2 additions & 1 deletion pkg/webhook/v1alpha2/applicationconfiguration/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/pkg/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand All @@ -31,7 +32,7 @@ func checkComponentVersionEnabled(ctx context.Context, client client.Reader, dm
return false, errors.Wrap(err, errUnmarshalTrait)
}
td, err := util.FetchTraitDefinition(ctx, client, dm, ut)
if err != nil {
if err != nil && !apierrors.IsNotFound(err) {
return false, errors.Wrapf(err, errFmtGetTraitDefinition, ut.GetAPIVersion(), ut.GetKind(), ut.GetName())
}
if td.Spec.RevisionEnabled {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e-test/component_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ var _ = Describe("Versioning mechanism of components", func() {
k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &comp1)
return comp1.Status.LatestRevision
},
time.Second*120, time.Millisecond*500).ShouldNot(BeNil())
time.Second*300, time.Millisecond*500).ShouldNot(BeNil())

revisionNameV1 := comp1.Status.LatestRevision.Name

Expand Down

0 comments on commit d992f48

Please sign in to comment.