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

Commit

Permalink
minor tweak, assign the workloadRef during render instead of apply (#249
Browse files Browse the repository at this point in the history 
)

* minor tweak, assign the workloadRef during render instead of apply

Signed-off-by: Ryan Zhang <[email protected]>

* remove debug

Signed-off-by: Ryan Zhang <[email protected]>
  • Loading branch information
@ryanzhang-oss
ryanzhang-oss authored Oct 15, 2020
1 parent 255734a commit c65ccab
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 73 deletions.
25 changes: 5 additions & 20 deletions pkg/controller/v1alpha2/applicationconfiguration/apply.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,35 +88,20 @@ func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus,
return errors.Wrapf(err, errFmtApplyWorkload, wl.Workload.GetName())
}
}

workloadRef := runtimev1alpha1.TypedReference{
APIVersion: wl.Workload.GetAPIVersion(),
Kind: wl.Workload.GetKind(),
Name: wl.Workload.GetName(),
}

for _, trait := range wl.Traits {
if trait.HasDep {
continue
}
// We only patch a TypedReference object to the trait if it asks for it
t := trait.Object
if traitDefinition, err := util.FetchTraitDefinition(ctx, a.rawClient, &trait.Object); err == nil {
workloadRefPath := traitDefinition.Spec.WorkloadRefPath
if len(workloadRefPath) != 0 {
if err := fieldpath.Pave(t.UnstructuredContent()).SetValue(workloadRefPath, workloadRef); err != nil {
return errors.Wrapf(err, errFmtSetWorkloadRef, t.GetName(), wl.Workload.GetName())
}
}
} else {
return errors.Wrapf(err, errFmtGetTraitDefinition, t.GetAPIVersion(), t.GetKind(), t.GetName())
}

if err := a.client.Apply(ctx, &trait.Object, ao...); err != nil {
return errors.Wrapf(err, errFmtApplyTrait, t.GetAPIVersion(), t.GetKind(), t.GetName())
}
}

workloadRef := runtimev1alpha1.TypedReference{
APIVersion: wl.Workload.GetAPIVersion(),
Kind: wl.Workload.GetKind(),
Name: wl.Workload.GetName(),
}
for _, s := range wl.Scopes {
if err := a.applyScope(ctx, wl, s, workloadRef); err != nil {
return err
Expand Down
52 changes: 0 additions & 52 deletions pkg/controller/v1alpha2/applicationconfiguration/apply_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ import (

func TestApplyWorkloads(t *testing.T) {
errBoom := errors.New("boom")
errTrait := errors.New("errTrait")

namespace := "ns"

workload := &unstructured.Unstructured{}
Expand Down Expand Up @@ -158,56 +156,6 @@ func TestApplyWorkloads(t *testing.T) {
ws: []v1alpha2.WorkloadStatus{}},
want: errors.Wrapf(errBoom, errFmtApplyTrait, trait.GetAPIVersion(), trait.GetKind(), trait.GetName()),
},
"GetTraitDefinitionError": {
reason: "Errors getting a traitDefinition should be reflected as a status condition",
rawClient: &test.MockClient{MockGet: test.NewMockGetFn(errTrait)},
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error {
return nil
}),
args: args{
w: []Workload{{Workload: workload, Traits: []*Trait{{Object: *trait}}}},
ws: []v1alpha2.WorkloadStatus{}},
want: errors.Wrapf(errTrait, errFmtGetTraitDefinition, trait.GetAPIVersion(), trait.GetKind(), trait.GetName()),
},
"TestApplyWorkloadRef": {
reason: "The workloadRef should be applied to a trait if its traitDefinition asks for it",
rawClient: &test.MockClient{MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
o, ok := obj.(*v1alpha2.TraitDefinition)
if ok {
td := v1alpha2.TraitDefinition{
Spec: v1alpha2.TraitDefinitionSpec{
WorkloadRefPath: "spec.workload.path",
},
}
*o = td
}
return nil
})},
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error {
if o.GetObjectKind().GroupVersionKind().Kind == trait.GetKind() {
// check if the trait has the workload ref
pavable, _ := util.Object2Map(o)
value, err := fieldpath.Pave(pavable).GetValue("spec.workload.path")
if err == nil {
wr, ok := value.(map[string]interface{})
if !ok {
return fmt.Errorf("didn't get the workload ref")
}
if wr["apiVersion"] != workload.GetAPIVersion() ||
wr["kind"] != workload.GetKind() || wr["name"] != workload.GetName() {
return fmt.Errorf("didn't get the right workload ref")
}

} else {
return fmt.Errorf("failed to apply the workload ref on %q with err = %+v", pavable["kind"], err)
}
}
return nil
}),
args: args{
w: []Workload{{Workload: workload, Traits: []*Trait{{Object: *trait.DeepCopy()}}}},
ws: []v1alpha2.WorkloadStatus{}},
},
"Success": {
reason: "Applied workloads and traits should be returned as a set of UIDs.",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error {
Expand Down
22 changes: 21 additions & 1 deletion pkg/controller/v1alpha2/applicationconfiguration/render.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ func (fn ComponentRenderFn) Render(ctx context.Context, ac *v1alpha2.Application
return fn(ctx, ac)
}

var _ ComponentRenderer = &components{}

type components struct {
client client.Reader
params ParameterResolver
Expand Down Expand Up @@ -149,12 +151,14 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati
traits := make([]*Trait, 0, len(acc.Traits))
traitDefs := make([]v1alpha2.TraitDefinition, 0, len(acc.Traits))
compInfoLabels[oam.LabelOAMResourceType] = oam.ResourceTypeTrait

for _, ct := range acc.Traits {
t, traitDef, err := r.renderTrait(ctx, ct, ac, acc.ComponentName, ref, dag)
if err != nil {
return nil, err
}
util.AddLabels(t, compInfoLabels)

// pass through labels and annotation from app-config to trait
util.PassLabelAndAnnotation(ac, t)
traits = append(traits, &Trait{Object: *t})
Expand All @@ -163,7 +167,23 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati
if err := SetWorkloadInstanceName(traitDefs, w, c); err != nil {
return nil, err
}

// create the ref after the workload name is set
workloadRef := runtimev1alpha1.TypedReference{
APIVersion: w.GetAPIVersion(),
Kind: w.GetKind(),
Name: w.GetName(),
}
// We only patch a TypedReference object to the trait if it asks for it
for i := range acc.Traits {
traitDef := traitDefs[i]
trait := traits[i]
workloadRefPath := traitDef.Spec.WorkloadRefPath
if len(workloadRefPath) != 0 {
if err := fieldpath.Pave(trait.Object.UnstructuredContent()).SetValue(workloadRefPath, workloadRef); err != nil {
return nil, errors.Wrapf(err, errFmtSetWorkloadRef, trait.Object.GetName(), w.GetName())
}
}
}
scopes := make([]unstructured.Unstructured, 0, len(acc.Scopes))
for _, cs := range acc.Scopes {
scopeObject, err := r.renderScope(ctx, cs, ac.GetNamespace())
Expand Down
130 changes: 130 additions & 0 deletions pkg/controller/v1alpha2/applicationconfiguration/render_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
"github.com/crossplane/crossplane-runtime/pkg/test"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -88,6 +89,7 @@ func TestRenderComponents(t *testing.T) {
},
}
ref := metav1.NewControllerRef(ac, v1alpha2.ApplicationConfigurationGroupVersionKind)
errTrait := errors.New("errTrait")

type fields struct {
client client.Reader
Expand Down Expand Up @@ -167,6 +169,38 @@ func TestRenderComponents(t *testing.T) {
err: errors.Wrapf(errBoom, errFmtRenderTrait, componentName),
},
},
"GetTraitDefinitionError": {
reason: "Errors getting a traitDefinition should be reflected as a status condition",
fields: fields{
client: &test.MockClient{MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
switch robj := obj.(type) {
case *v1alpha2.Component:
ccomp := v1alpha2.Component{Status: v1alpha2.ComponentStatus{LatestRevision: &v1alpha2.Revision{Name: revisionName2}}}
ccomp.DeepCopyInto(robj)
case *v1alpha2.TraitDefinition:
return errTrait
}
return nil
})},
params: ParameterResolveFn(func(_ []v1alpha2.ComponentParameter, _ []v1alpha2.ComponentParameterValue) ([]Parameter, error) {
return nil, nil
}),
workload: ResourceRenderFn(func(_ []byte, _ ...Parameter) (*unstructured.Unstructured, error) {
return &unstructured.Unstructured{}, nil
}),
trait: ResourceRenderFn(func(_ []byte, _ ...Parameter) (*unstructured.Unstructured, error) {
t := &unstructured.Unstructured{}
t.SetName(traitName)
t.SetAPIVersion("traitAPI")
t.SetKind("traitKind")
return t, nil
}),
},
args: args{ac: ac},
want: want{
err: errors.Wrapf(errTrait, errFmtGetTraitDefinition, "traitAPI", "traitKind", traitName),
},
},
"Success": {
reason: "One workload and one trait should successfully be rendered",
fields: fields{
Expand Down Expand Up @@ -365,6 +399,102 @@ func TestRenderComponents(t *testing.T) {
},
},
},
"Success-With-WorkloadRef": {
reason: "Workload should successfully be rendered with fixed componentRevision",
fields: fields{
client: &test.MockClient{MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
robj, ok := obj.(*v1.ControllerRevision)
if ok {
rev := &v1.ControllerRevision{
ObjectMeta: metav1.ObjectMeta{Name: revisionName, Namespace: namespace},
Data: runtime.RawExtension{Object: &v1alpha2.Component{
ObjectMeta: metav1.ObjectMeta{
Name: componentName,
Namespace: namespace,
},
Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &unstructured.Unstructured{}}},
Status: v1alpha2.ComponentStatus{},
}},
Revision: 1,
}
rev.DeepCopyInto(robj)
return nil
}
trd, ok := obj.(*v1alpha2.TraitDefinition)
if ok {
td := v1alpha2.TraitDefinition{
Spec: v1alpha2.TraitDefinitionSpec{
WorkloadRefPath: "spec.workload.path",
},
}
td.DeepCopyInto(trd)
}
return nil
})},
params: ParameterResolveFn(func(_ []v1alpha2.ComponentParameter, _ []v1alpha2.ComponentParameterValue) ([]Parameter, error) {
return nil, nil
}),
workload: ResourceRenderFn(func(_ []byte, _ ...Parameter) (*unstructured.Unstructured, error) {
w := &unstructured.Unstructured{}
w.SetAPIVersion("traitApiVersion")
w.SetKind("traitKind")
return w, nil
}),
trait: ResourceRenderFn(func(_ []byte, _ ...Parameter) (*unstructured.Unstructured, error) {
t := &unstructured.Unstructured{}
t.SetName(traitName)
return t, nil
}),
},
args: args{ac: revAC},
want: want{
w: []Workload{
{
ComponentName: componentName,
ComponentRevisionName: revisionName,
Workload: func() *unstructured.Unstructured {
w := &unstructured.Unstructured{}
w.SetAPIVersion("traitApiVersion")
w.SetKind("traitKind")
w.SetNamespace(namespace)
w.SetName(componentName)
w.SetOwnerReferences([]metav1.OwnerReference{*ref})
w.SetLabels(map[string]string{
oam.LabelAppComponent: componentName,
oam.LabelAppName: acName,
oam.LabelAppComponentRevision: revisionName,
oam.LabelOAMResourceType: oam.ResourceTypeWorkload,
})
return w
}(),
Traits: []*Trait{
func() *Trait {
tr := &unstructured.Unstructured{}
tr.SetNamespace(namespace)
tr.SetName(traitName)
tr.SetOwnerReferences([]metav1.OwnerReference{*ref})
tr.SetLabels(map[string]string{
oam.LabelAppComponent: componentName,
oam.LabelAppName: acName,
oam.LabelAppComponentRevision: revisionName,
oam.LabelOAMResourceType: oam.ResourceTypeTrait,
})
workloadRef := runtimev1alpha1.TypedReference{
APIVersion: "traitApiVersion",
Kind: "traitKind",
Name: componentName,
}
if err := fieldpath.Pave(tr.Object).SetValue("spec.workload.path", workloadRef); err != nil {
t.Fail()
}
return &Trait{Object: *tr}
}(),
},
Scopes: []unstructured.Unstructured{},
},
},
},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
Expand Down

0 comments on commit c65ccab

Please sign in to comment.