Skip to content

Commit

Permalink
add unit test for apicompatible
Browse files Browse the repository at this point in the history
also fix review comments

Signed-off-by: Jian Qiu <jqiu@redhat.com>
  • Loading branch information
qiujian16 committed Aug 19, 2022
1 parent 5f26447 commit 5facb25
Show file tree
Hide file tree
Showing 5 changed files with 304 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type apiCompatibleReconciler struct {
listAPIResourceImports func(clusterName logicalcluster.Name) ([]*apiresourcev1alpha1.APIResourceImport, error)
}

func (e *apiCompatibleReconciler) reconcile(ctx context.Context, syncTarget *workloadv1alpha1.SyncTarget) (*workloadv1alpha1.SyncTarget, reconcileStatus, error) {
func (e *apiCompatibleReconciler) reconcile(ctx context.Context, syncTarget *workloadv1alpha1.SyncTarget) (*workloadv1alpha1.SyncTarget, error) {
exportKeys := getExportKeys(syncTarget)

var errs []error
Expand Down Expand Up @@ -87,7 +87,7 @@ func (e *apiCompatibleReconciler) reconcile(ctx context.Context, syncTarget *wor
apiImportMap := map[schema.GroupVersionResource]*apiextensionsv1.JSONSchemaProps{}
apiImports, err := e.listAPIResourceImports(lcluster)
if err != nil {
return syncTarget, reconcileStatusStop, err
return syncTarget, err
}

for _, apiImport := range apiImports {
Expand Down Expand Up @@ -125,11 +125,11 @@ func (e *apiCompatibleReconciler) reconcile(ctx context.Context, syncTarget *wor
continue
}

// since version is ordered, so if the current version is comptaible, we can skipp the check on other versions.
// since version is ordered, so if the current version is comptaible, we can skip the check on other versions.
syncTarget.Status.SyncedResources[i].State = workloadv1alpha1.ResourceSchemaAcceptedState
break
}
}

return syncTarget, reconcileStatusContinue, errors.NewAggregate(errs)
return syncTarget, errors.NewAggregate(errs)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
/*
Copyright 2022 The KCP Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package synctargetexports

import (
"context"
"testing"

"github.com/kcp-dev/logicalcluster/v2"
"github.com/stretchr/testify/require"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"

apiresourcev1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apiresource/v1alpha1"
apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
workloadv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/workload/v1alpha1"
)

func TestSyncTargetCompatibleReconcile(t *testing.T) {
tests := []struct {
name string
syncTarget *workloadv1alpha1.SyncTarget
export *apisv1alpha1.APIExport
schemas []*apisv1alpha1.APIResourceSchema
apiResourceImport []*apiresourcev1alpha1.APIResourceImport

wantError bool
wantSyncedResources []workloadv1alpha1.ResourceToSync
}{
{
name: "pending when missing APIResourceSchema",
syncTarget: newSyncTarget([]apisv1alpha1.ExportReference{
{
Workspace: &apisv1alpha1.WorkspaceExportReference{ExportName: "kubernetes"},
}},
[]workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaAcceptedState},
},
),
export: newAPIExport("kubernetes", []string{"apps.v1.deployment"}, ""),
wantSyncedResources: []workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaPendingState},
},
},
{
name: "incompatible when missing APIResourceImport",
syncTarget: newSyncTarget([]apisv1alpha1.ExportReference{
{
Workspace: &apisv1alpha1.WorkspaceExportReference{ExportName: "kubernetes"},
}},
[]workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaAcceptedState},
},
),
export: newAPIExport("kubernetes", []string{"apps.v1.deployment"}, ""),
schemas: []*apisv1alpha1.APIResourceSchema{
newResourceSchema("apps.v1.deployment", "apps", "deployments", []apisv1alpha1.APIResourceVersion{
{
Name: "v1",
Served: true,
Schema: runtime.RawExtension{Raw: []byte(`{"type":"string"}`)},
},
}),
},
wantSyncedResources: []workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaIncomptibleState},
},
},
{
name: "APIResourceImport compatible with APIResourceSchema",
syncTarget: newSyncTarget([]apisv1alpha1.ExportReference{
{
Workspace: &apisv1alpha1.WorkspaceExportReference{ExportName: "kubernetes"},
}},
[]workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaPendingState},
},
),
export: newAPIExport("kubernetes", []string{"apps.v1.deployment"}, ""),
schemas: []*apisv1alpha1.APIResourceSchema{
newResourceSchema("apps.v1.deployment", "apps", "deployments", []apisv1alpha1.APIResourceVersion{
{
Name: "v1",
Served: true,
Schema: runtime.RawExtension{Raw: []byte(`{"type":"string"}`)},
},
}),
},
apiResourceImport: []*apiresourcev1alpha1.APIResourceImport{
newAPIResourceImport("apps.v1.deployment", "apps", "deployments", "v1", `{"type":"string"}`),
},
wantSyncedResources: []workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaAcceptedState},
},
},
{
name: "APIResourceImport incompatible with APIResourceSchema",
syncTarget: newSyncTarget([]apisv1alpha1.ExportReference{
{
Workspace: &apisv1alpha1.WorkspaceExportReference{ExportName: "kubernetes"},
}},
[]workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaAcceptedState},
},
),
export: newAPIExport("kubernetes", []string{"apps.v1.deployment"}, ""),
schemas: []*apisv1alpha1.APIResourceSchema{
newResourceSchema("apps.v1.deployment", "apps", "deployments", []apisv1alpha1.APIResourceVersion{
{
Name: "v1",
Served: true,
Schema: runtime.RawExtension{Raw: []byte(`{"type":"integer"}`)},
},
}),
},
apiResourceImport: []*apiresourcev1alpha1.APIResourceImport{
newAPIResourceImport("apps.v1.deployment", "apps", "deployments", "v1", `{"type":"string"}`),
},
wantSyncedResources: []workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1"}, State: workloadv1alpha1.ResourceSchemaIncomptibleState},
},
},
{
name: "only take care latest version",
syncTarget: newSyncTarget([]apisv1alpha1.ExportReference{
{
Workspace: &apisv1alpha1.WorkspaceExportReference{ExportName: "kubernetes"},
}},
[]workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1", "v1beta1"}, State: workloadv1alpha1.ResourceSchemaPendingState},
},
),
export: newAPIExport("kubernetes", []string{"apps.v1.deployment"}, ""),
schemas: []*apisv1alpha1.APIResourceSchema{
newResourceSchema("apps.v1.deployment", "apps", "deployments", []apisv1alpha1.APIResourceVersion{
{
Name: "v1",
Served: true,
Schema: runtime.RawExtension{Raw: []byte(`{"type":"string"}`)},
},
}),
newResourceSchema("apps.v1.deployment", "apps", "deployments", []apisv1alpha1.APIResourceVersion{
{
Name: "v1beta1",
Served: true,
Schema: runtime.RawExtension{Raw: []byte(`{"type":"string"}`)},
},
}),
},
apiResourceImport: []*apiresourcev1alpha1.APIResourceImport{
newAPIResourceImport("apps.v1.deployment", "apps", "deployments", "v1", `{"type":"string"}`),
},
wantSyncedResources: []workloadv1alpha1.ResourceToSync{
{GroupResource: apisv1alpha1.GroupResource{Group: "apps", Resource: "deployments"}, Versions: []string{"v1", "v1beta1"}, State: workloadv1alpha1.ResourceSchemaAcceptedState},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
getAPIExport := func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExport, error) {
if tc.export == nil {
return nil, errors.NewNotFound(schema.GroupResource{}, name)
}
return tc.export, nil
}
getResourceSchema := func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIResourceSchema, error) {
for _, schema := range tc.schemas {
if schema.Name == name {
return schema, nil
}
}

return nil, errors.NewNotFound(schema.GroupResource{}, name)
}
listAPIResourceImports := func(clusterName logicalcluster.Name) ([]*apiresourcev1alpha1.APIResourceImport, error) {
return tc.apiResourceImport, nil
}

reconciler := &apiCompatibleReconciler{
getAPIExport: getAPIExport,
getResourceSchema: getResourceSchema,
listAPIResourceImports: listAPIResourceImports,
}

updated, err := reconciler.reconcile(context.TODO(), tc.syncTarget)
if tc.wantError {
require.Error(t, err)
} else {
require.NoError(t, err)
}

require.Equal(t, tc.wantSyncedResources, updated.Status.SyncedResources)
})
}
}

func newAPIResourceImport(name, group, resource, version, schema string) *apiresourcev1alpha1.APIResourceImport {
return &apiresourcev1alpha1.APIResourceImport{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Spec: apiresourcev1alpha1.APIResourceImportSpec{
CommonAPIResourceSpec: apiresourcev1alpha1.CommonAPIResourceSpec{
GroupVersion: apiresourcev1alpha1.GroupVersion{
Group: group,
Version: version,
},
CustomResourceDefinitionNames: apiextensionsv1.CustomResourceDefinitionNames{
Plural: resource,
},
OpenAPIV3Schema: runtime.RawExtension{Raw: []byte(schema)},
},
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/tools/cache"
Expand All @@ -36,6 +37,7 @@ import (
"k8s.io/klog/v2"

apiresourcev1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apiresource/v1alpha1"
apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
workloadv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/workload/v1alpha1"
kcpclient "github.com/kcp-dev/kcp/pkg/client/clientset/versioned"
apiresourceinformer "github.com/kcp-dev/kcp/pkg/client/informers/externalversions/apiresource/v1alpha1"
Expand All @@ -49,9 +51,9 @@ import (
const (
controllerName = "kcp-synctarget-export-controller"

indexSyncTargetsbyExport = controllerName + "ByExport"
indexSyncTargetsByExport = controllerName + "ByExport"
indexAPIExportsByAPIResourceSchema = controllerName + "ByAPIResourceSchema"
indexbyWorkspace = controllerName + "ByWorkspace" // will go away with scoping
indexByWorkspace = controllerName + "ByWorkspace" // will go away with scoping
)

// NewController returns a controller which update syncedResource in status based on supportedExports in spec
Expand All @@ -77,7 +79,7 @@ func NewController(
}

if err := syncTargetInformer.Informer().AddIndexers(cache.Indexers{
indexSyncTargetsbyExport: indexSyncTargetsByExports,
indexSyncTargetsByExport: indexSyncTargetsByExports,
}); err != nil {
return nil, err
}
Expand All @@ -89,7 +91,7 @@ func NewController(
}

if err := apiResourceImportInformer.Informer().AddIndexers(cache.Indexers{
indexbyWorkspace: indexByWorksapce,
indexByWorkspace: indexByWorksapce,
}); err != nil {
return nil, err
}
Expand Down Expand Up @@ -123,14 +125,14 @@ func NewController(
})

apiResourceImportInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: c.enqueueAPIImport,
AddFunc: c.enqueueAPIResourceImport,
UpdateFunc: func(old, obj interface{}) {
oldImport := old.(*apiresourcev1alpha1.APIResourceImport)
newImport := obj.(*apiresourcev1alpha1.APIResourceImport)

// only enqueue when spec is changed.
if oldImport.Generation != newImport.Generation {
c.enqueueSyncTarget(obj, "")
c.enqueueAPIResourceImport(obj)
}
},
DeleteFunc: func(obj interface{}) {},
Expand Down Expand Up @@ -163,7 +165,7 @@ func (c *Controller) enqueueSyncTarget(obj interface{}, logSuffix string) {
c.queue.Add(key)
}

func (c *Controller) enqueueAPIImport(obj interface{}) {
func (c *Controller) enqueueAPIResourceImport(obj interface{}) {
apiImport, ok := obj.(*apiresourcev1alpha1.APIResourceImport)
if !ok {
runtime.HandleError(fmt.Errorf("obj is supposed to be a APIResourceImport, but is %T", obj))
Expand All @@ -184,7 +186,7 @@ func (c *Controller) enqueueAPIExport(obj interface{}, logSuffix string) {
return
}

synctargets, err := c.syncTargetIndexer.ByIndex(indexSyncTargetsbyExport, key)
synctargets, err := c.syncTargetIndexer.ByIndex(indexSyncTargetsByExport, key)
if err != nil {
runtime.HandleError(err)
return
Expand Down Expand Up @@ -313,3 +315,50 @@ func (c *Controller) process(ctx context.Context, key string) error {

return nil
}

func (c *Controller) reconcile(ctx context.Context, syncTarget *workloadv1alpha1.SyncTarget) (*workloadv1alpha1.SyncTarget, error) {
var errs []error

exportReconciler := &exportReconciler{
getAPIExport: c.getAPIExport,
getResourceSchema: c.getResourceSchema,
}
syncTarget, err := exportReconciler.reconcile(ctx, syncTarget)
if err != nil {
errs = append(errs, err)
}

apiCompatibleReconciler := &apiCompatibleReconciler{
getAPIExport: c.getAPIExport,
getResourceSchema: c.getResourceSchema,
listAPIResourceImports: c.listAPIResourceImports,
}
syncTarget, err = apiCompatibleReconciler.reconcile(ctx, syncTarget)
if err != nil {
errs = append(errs, err)
}

return syncTarget, errors.NewAggregate(errs)
}

func (c *Controller) getAPIExport(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExport, error) {
key := clusters.ToClusterAwareKey(clusterName, name)
return c.apiExportLister.Get(key)
}

func (c *Controller) getResourceSchema(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIResourceSchema, error) {
key := clusters.ToClusterAwareKey(clusterName, name)
return c.resourceSchemaLister.Get(key)
}

func (c *Controller) listAPIResourceImports(clusterName logicalcluster.Name) ([]*apiresourcev1alpha1.APIResourceImport, error) {
items, err := c.apiImportIndexer.ByIndex(indexByWorkspace, clusterName.String())
if err != nil {
return nil, err
}
ret := make([]*apiresourcev1alpha1.APIResourceImport, 0, len(items))
for _, item := range items {
ret = append(ret, item.(*apiresourcev1alpha1.APIResourceImport))
}
return ret, nil
}
Loading

0 comments on commit 5facb25

Please sign in to comment.