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

Commit

Permalink
use updating-apply instead of patching-apply on creating/updating trait
Browse files Browse the repository at this point in the history 
upgrade crossplane/crossplane-runtime 0.8.0=>0.10.0

add e2e-test

Signed-off-by: roy wang <[email protected]>
  • Loading branch information
@captainroy-hy
captainroy-hy committed Nov 7, 2020
1 parent 7a6a94a commit f11ab7c
Show file tree
Hide file tree
Showing 8 changed files with 399 additions and 60 deletions.
26 changes: 16 additions & 10 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,33 @@ module github.com/crossplane/oam-kubernetes-runtime
go 1.13

require (
github.com/crossplane/crossplane-runtime v0.8.0
github.com/Azure/go-autorest/autorest v0.9.2 // indirect
github.com/Azure/go-autorest/autorest/adal v0.8.0 // indirect
github.com/crossplane/crossplane-runtime v0.10.0
github.com/davecgh/go-spew v1.1.1
github.com/ghodss/yaml v1.0.0
github.com/go-logr/logr v0.1.0
github.com/go-logr/zapr v0.1.1 // indirect
github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6 // indirect
github.com/google/go-cmp v0.4.0
github.com/json-iterator/go v1.1.8
github.com/onsi/ginkgo v1.11.0
github.com/onsi/gomega v1.8.1
github.com/gophercloud/gophercloud v0.6.0 // indirect
github.com/json-iterator/go v1.1.10
github.com/onsi/ginkgo v1.12.1
github.com/onsi/gomega v1.10.1
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.1.0 // indirect
github.com/stretchr/testify v1.4.0
go.uber.org/zap v1.10.0
golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0
k8s.io/api v0.18.5
k8s.io/apiextensions-apiserver v0.18.2
k8s.io/apimachinery v0.18.5
k8s.io/client-go v0.18.5
k8s.io/api v0.18.6
k8s.io/apiextensions-apiserver v0.18.6
k8s.io/apimachinery v0.18.6
k8s.io/client-go v0.18.6
k8s.io/klog v1.0.0
k8s.io/kube-openapi v0.0.0-20200410145947-61e04a5be9a6
k8s.io/kubectl v0.18.5
k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89
sigs.k8s.io/controller-runtime v0.6.0
k8s.io/utils v0.0.0-20200603063816-c1c6865ac451
sigs.k8s.io/controller-runtime v0.6.2
sigs.k8s.io/controller-tools v0.2.4
)
86 changes: 62 additions & 24 deletions go.sum
View file

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,11 @@ func NewReconciler(m ctrl.Manager, dm discoverymapper.DiscoveryMapper, o ...Reco
trait: ResourceRenderFn(renderTrait),
},
workloads: &workloads{
client: resource.NewAPIPatchingApplicator(m.GetClient()),
rawClient: m.GetClient(),
dm: dm,
// NOTE(roywang) [email protected] only use "application/merge-patch+json" type patch
patchingClient: resource.NewAPIPatchingApplicator(m.GetClient()),
updatingClient: resource.NewAPIUpdatingApplicator(m.GetClient()),
rawClient: m.GetClient(),
dm: dm,
},
gc: GarbageCollectorFn(eligible),
log: logging.NewNopLogger(),
Expand Down
13 changes: 8 additions & 5 deletions pkg/controller/v1alpha2/applicationconfiguration/apply.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,20 @@ func (fn WorkloadApplyFns) Finalize(ctx context.Context, ac *v1alpha2.Applicatio
}

type workloads struct {
client resource.Applicator
rawClient client.Client
dm discoverymapper.DiscoveryMapper
// use patching-apply for creating/updating Workload
patchingClient resource.Applicator
// use updateing-apply for creating/updating Trait
updatingClient resource.Applicator
rawClient client.Client
dm discoverymapper.DiscoveryMapper
}

func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus, w []Workload, ao ...resource.ApplyOption) error {
// they are all in the same namespace
var namespace = w[0].Workload.GetNamespace()
for _, wl := range w {
if !wl.HasDep {
err := a.client.Apply(ctx, wl.Workload, ao...)
err := a.patchingClient.Apply(ctx, wl.Workload, ao...)
if err != nil {
return errors.Wrapf(err, errFmtApplyWorkload, wl.Workload.GetName())
}
Expand All @@ -95,7 +98,7 @@ func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus,
continue
}
t := trait.Object
if err := a.client.Apply(ctx, &trait.Object, ao...); err != nil {
if err := a.updatingClient.Apply(ctx, &trait.Object, ao...); err != nil {
return errors.Wrapf(err, errFmtApplyTrait, t.GetAPIVersion(), t.GetKind(), t.GetName())
}
}
Expand Down
39 changes: 23 additions & 16 deletions pkg/controller/v1alpha2/applicationconfiguration/apply_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,12 @@ func TestApplyWorkloads(t *testing.T) {
}

cases := map[string]struct {
reason string
client resource.Applicator
rawClient client.Client
args args
want error
reason string
client resource.Applicator
updatingClient resource.Applicator
rawClient client.Client
args args
want error
}{
"ApplyWorkloadError": {
reason: "Errors applying a workload should be reflected as a status condition",
Expand All @@ -137,15 +138,17 @@ func TestApplyWorkloads(t *testing.T) {
}
return nil
}),
rawClient: nil,
updatingClient: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
rawClient: nil,
args: args{
w: []Workload{{Workload: workload, Traits: []*Trait{{Object: *trait}}}},
ws: []v1alpha2.WorkloadStatus{}},
want: errors.Wrapf(errBoom, errFmtApplyWorkload, workload.GetName()),
},
"ApplyTraitError": {
reason: "Errors applying a trait should be reflected as a status condition",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error {
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
updatingClient: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error {
if t, ok := o.(*unstructured.Unstructured); ok && t.GetUID() == trait.GetUID() {
return errBoom
}
Expand All @@ -159,7 +162,8 @@ func TestApplyWorkloads(t *testing.T) {
},
"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 {
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
updatingClient: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error {
if o.GetObjectKind().GroupVersionKind().Kind == trait.GetKind() {
// check that the trait should not have a workload ref since we didn't return a special traitDefinition
obj, _ := util.Object2Map(o)
Expand All @@ -176,8 +180,9 @@ func TestApplyWorkloads(t *testing.T) {
},
},
"SuccessWithScope": {
reason: "Applied workloads refs to scopes.",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
reason: "Applied workloads refs to scopes.",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
updatingClient: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
rawClient: &test.MockClient{
MockGet: func(_ context.Context, key client.ObjectKey, obj runtime.Object) error {
if scopeDef, ok := obj.(*v1alpha2.ScopeDefinition); ok {
Expand Down Expand Up @@ -217,8 +222,9 @@ func TestApplyWorkloads(t *testing.T) {
},
},
"SuccessWithScopeNoOp": {
reason: "Scope already has workloadRef.",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
reason: "Scope already has workloadRef.",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
updatingClient: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
rawClient: &test.MockClient{
MockGet: func(_ context.Context, key client.ObjectKey, obj runtime.Object) error {
if scopeDef, ok := obj.(*v1alpha2.ScopeDefinition); ok {
Expand Down Expand Up @@ -258,8 +264,9 @@ func TestApplyWorkloads(t *testing.T) {
},
},
"SuccessRemoving": {
reason: "Removes workload refs from scopes.",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
reason: "Removes workload refs from scopes.",
client: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
updatingClient: resource.ApplyFn(func(_ context.Context, o runtime.Object, _ ...resource.ApplyOption) error { return nil }),
rawClient: &test.MockClient{
MockGet: func(_ context.Context, key client.ObjectKey, obj runtime.Object) error {
if key.Name == scope.GetName() {
Expand Down Expand Up @@ -321,7 +328,7 @@ func TestApplyWorkloads(t *testing.T) {
t.Run(name, func(t *testing.T) {
mapper := mock.NewMockDiscoveryMapper()

w := workloads{client: tc.client, rawClient: tc.rawClient, dm: mapper}
w := workloads{patchingClient: tc.client, updatingClient: tc.updatingClient, rawClient: tc.rawClient, dm: mapper}
err := w.Apply(tc.args.ctx, tc.args.ws, tc.args.w)

if diff := cmp.Diff(tc.want, err, test.EquateErrors()); diff != "" {
Expand Down Expand Up @@ -467,7 +474,7 @@ func TestFinalizeWorkloadScopes(t *testing.T) {
t.Run(tc.caseName, func(t *testing.T) {
acTest := ac
mapper := mock.NewMockDiscoveryMapper()
w := workloads{client: tc.client, rawClient: tc.rawClient, dm: mapper}
w := workloads{patchingClient: tc.client, rawClient: tc.rawClient, dm: mapper}
err := w.Finalize(ctx, &acTest)

if diff := cmp.Diff(tc.wantErr, err, test.EquateErrors()); diff != "" {
Expand Down
14 changes: 14 additions & 0 deletions pkg/controller/v1alpha2/applicationconfiguration/dependency_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,20 @@ var _ = Describe("Resource Dependency in an ApplicationConfiguration", func() {
logf.Log.Info("Checking on resource that inputs data", "Key", inFooKey)
Expect(k8sClient.Get(ctx, inFooKey, inFoo)).Should(BeNil())

Eventually(func() error {
err := k8sClient.Get(ctx, outFooKey, outFoo)
if err != nil {
// Try 3 (= 1s/300ms) times
reconciler.Reconcile(req)
}
return err
}, time.Second, 300*time.Millisecond).Should(BeNil())
err = unstructured.SetNestedField(outFoo.Object, "test", "status", "key")
Expect(err).Should(BeNil())
err = unstructured.SetNestedField(outFoo.Object, "hash-v1", "status", "app-hash")
Expect(err).Should(BeNil())
Expect(k8sClient.Update(ctx, outFoo)).Should(Succeed())

By("Update AppConfig with new version")
newAppConfig.Labels["app-hash"] = "hash-v2"
Expect(k8sClient.Update(ctx, newAppConfig)).Should(BeNil())
Expand Down
Loading

0 comments on commit f11ab7c

Please sign in to comment.