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

minor tweak, assign the workloadRef during render instead of apply #249

Merged
merged 2 commits into from
Oct 15, 2020
Merged

minor tweak, assign the workloadRef during render instead of apply #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ef9849
minor tweak, assign the workloadRef during render instead of apply
ryanzhang-oss Oct 15, 2020
751f6d9
remove debug
ryanzhang-oss Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 5 additions & 20 deletions pkg/controller/v1alpha2/applicationconfiguration/apply.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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