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

Commit

Permalink
refactor and fix test
Browse files Browse the repository at this point in the history
Signed-off-by: 天元 <[email protected]>
  • Loading branch information
wonderflow committed Oct 25, 2020
1 parent 7b827e5 commit 689bc39
Show file tree
Hide file tree
Showing 30 changed files with 810 additions and 829 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ jobs:
with:
go-version: ${{ env.GO_VERSION }}

- name: install Kubebuilder
uses: RyanSiu1995/kubebuilder-action@v1

- name: Cache Go Dependencies
uses: actions/cache@v2
with:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.4.0
go.uber.org/zap v1.10.0
golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,10 @@ import (
"time"

"github.com/pkg/errors"
meta2 "k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

Expand All @@ -43,6 +41,7 @@ import (
"github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2"
"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"
)

const (
Expand Down Expand Up @@ -81,9 +80,9 @@ const (

// Setup adds a controller that reconciles ApplicationConfigurations.
func Setup(mgr ctrl.Manager, args controller.Args, l logging.Logger) error {
mapper, err := apiutil.NewDiscoveryRESTMapper(mgr.GetConfig())
dm, err := discoverymapper.New(mgr.GetConfig())
if err != nil {
return fmt.Errorf("create discovery mapper fail %v", err)
return fmt.Errorf("create discovery dm fail %v", err)
}
name := "oam/" + strings.ToLower(v1alpha2.ApplicationConfigurationGroupKind)

Expand All @@ -95,7 +94,7 @@ func Setup(mgr ctrl.Manager, args controller.Args, l logging.Logger) error {
Logger: l,
RevisionLimit: args.RevisionLimit,
}).
Complete(NewReconciler(mgr, mapper,
Complete(NewReconciler(mgr, dm,
WithLogger(l.WithValues("controller", name)),
WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name)))))
}
Expand Down Expand Up @@ -170,21 +169,21 @@ func WithPosthook(name string, hook ControllerHooks) ReconcilerOption {

// NewReconciler returns an OAMApplicationReconciler that reconciles ApplicationConfigurations
// by rendering and instantiating their Components and Traits.
func NewReconciler(m ctrl.Manager, mapper meta2.RESTMapper, o ...ReconcilerOption) *OAMApplicationReconciler {
func NewReconciler(m ctrl.Manager, dm discoverymapper.DiscoveryMapper, o ...ReconcilerOption) *OAMApplicationReconciler {
r := &OAMApplicationReconciler{
client: m.GetClient(),
scheme: m.GetScheme(),
components: &components{
client: m.GetClient(),
mapper: mapper,
dm: dm,
params: ParameterResolveFn(resolve),
workload: ResourceRenderFn(renderWorkload),
trait: ResourceRenderFn(renderTrait),
},
workloads: &workloads{
client: resource.NewAPIPatchingApplicator(m.GetClient()),
rawClient: m.GetClient(),
mapper: mapper,
dm: dm,
},
gc: GarbageCollectorFn(eligible),
log: logging.NewNopLogger(),
Expand Down Expand Up @@ -269,7 +268,7 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco

workloads, depStatus, err := r.components.Render(ctx, ac)
if err != nil {
log.Debug("Cannot render components", "error", err, "requeue-after", time.Now().Add(shortWait))
log.Info("Cannot render components", "error", err, "requeue-after", time.Now().Add(shortWait))
r.record.Event(ac, event.Warning(reasonCannotRenderComponents, err))
ac.SetConditions(v1alpha1.ReconcileError(errors.Wrap(err, errRenderComponents)))
return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, ac), errUpdateAppConfigStatus)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,7 @@ func TestDependency(t *testing.T) {
t.Fatal(err)
}

mapper := mock.NewMockMapper()
mapper := mock.NewMockDiscoveryMapper()

type args struct {
components []v1alpha2.ApplicationConfigurationComponent
Expand Down Expand Up @@ -1299,7 +1299,7 @@ func TestDependency(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
c := components{
mapper: mapper,
dm: mapper,
client: &test.MockClient{
MockGet: test.MockGetFn(func(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
if obj.GetObjectKind().GroupVersionKind().Kind == "Workload" {
Expand Down
9 changes: 4 additions & 5 deletions pkg/controller/v1alpha2/applicationconfiguration/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ package applicationconfiguration
import (
"context"

meta2 "k8s.io/apimachinery/pkg/api/meta"

runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
"github.com/crossplane/crossplane-runtime/pkg/meta"
Expand All @@ -32,6 +30,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

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

Expand Down Expand Up @@ -78,7 +77,7 @@ func (fn WorkloadApplyFns) Finalize(ctx context.Context, ac *v1alpha2.Applicatio
type workloads struct {
client resource.Applicator
rawClient client.Client
mapper meta2.RESTMapper
dm discoverymapper.DiscoveryMapper
}

func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus, w []Workload, ao ...resource.ApplyOption) error {
Expand Down Expand Up @@ -186,7 +185,7 @@ func findDereferencedScopes(statusScopes []v1alpha2.WorkloadScope, scopes []unst

func (a *workloads) applyScope(ctx context.Context, wl Workload, s unstructured.Unstructured, workloadRef runtimev1alpha1.TypedReference) error {
// get ScopeDefinition
scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.mapper, &s)
scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.dm, &s)
if err != nil {
return errors.Wrapf(err, errFmtGetScopeDefinition, s.GetAPIVersion(), s.GetKind(), s.GetName())
}
Expand Down Expand Up @@ -240,7 +239,7 @@ func (a *workloads) applyScopeRemoval(ctx context.Context, namespace string, wr
return errors.Wrapf(err, errFmtApplyScope, s.Reference.APIVersion, s.Reference.Kind, s.Reference.Name)
}

scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.mapper, &scopeObject)
scopeDefinition, err := util.FetchScopeDefinition(ctx, a.rawClient, a.dm, &scopeObject)
if err != nil {
return errors.Wrapf(err, errFmtGetScopeDefinition, scopeObject.GetAPIVersion(), scopeObject.GetKind(), scopeObject.GetName())
}
Expand Down
11 changes: 5 additions & 6 deletions pkg/controller/v1alpha2/applicationconfiguration/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"fmt"
"testing"

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

"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
"github.com/crossplane/crossplane-runtime/pkg/resource"
Expand All @@ -36,6 +34,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

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

Expand Down Expand Up @@ -320,9 +319,9 @@ func TestApplyWorkloads(t *testing.T) {

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
mapper := mock.NewMockMapper()
mapper := mock.NewMockDiscoveryMapper()

w := workloads{client: tc.client, rawClient: tc.rawClient, mapper: mapper}
w := workloads{client: tc.client, 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,8 +466,8 @@ func TestFinalizeWorkloadScopes(t *testing.T) {
for _, tc := range cases {
t.Run(tc.caseName, func(t *testing.T) {
acTest := ac
mapper := mock.NewMockMapper()
w := workloads{client: tc.client, rawClient: tc.rawClient, mapper: mapper}
mapper := mock.NewMockDiscoveryMapper()
w := workloads{client: tc.client, rawClient: tc.rawClient, dm: mapper}
err := w.Finalize(ctx, &acTest)

if diff := cmp.Diff(tc.wantErr, err, test.EquateErrors()); diff != "" {
Expand Down
Loading

0 comments on commit 689bc39

Please sign in to comment.