Skip to content

Commit

Permalink
Always populate TemplateParams 'Object' (#249)
Browse files Browse the repository at this point in the history
## Change Overview

This commit modifies template params creation to always
populate the Object field with the unstructured representation
of the Kubernetes object that is the target of the actionset

This allows writing generic blueprints that can be re-used
across different types of Kubernetes resources and to reference
fields from within the object spec that we do not hoist into
TemplateParams e.g. `.Object.metadata.labels.<foo>`

The change described is contained to `param.go` and `param_test.go`.
The rest of the changes are to the `kube` pkg to allow testing with
a fake dynamic client. 

## Pull request type

Please check the type of change your PR introduces:
- [ ] Work in Progress
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Trival/Minor
- [x] Bugfix
- [ ] Feature
- [ ] Documentation

## Issues

- #247 

## Test Plan

- [ ] Manual
- [x] Unit test
- [ ] E2E

```
$ go test -check.vv
...
OK: 14 passed
PASS
ok  	github.com/kanisterio/kanister/pkg/param	103.201s
```
  • Loading branch information
Vaibhav Kamra authored Aug 28, 2019
1 parent c9542f7 commit da43bcd
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 35 deletions.
10 changes: 9 additions & 1 deletion pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
Expand All @@ -54,6 +55,7 @@ type Controller struct {
config *rest.Config
crClient versioned.Interface
clientset kubernetes.Interface
dynClient dynamic.Interface
recorder record.EventRecorder
actionSetTombMap sync.Map
}
Expand All @@ -78,8 +80,14 @@ func (c *Controller) StartWatch(ctx context.Context, namespace string) error {
if err != nil {
return errors.Wrap(err, "failed to get a k8s client")
}
dynClient, err := dynamic.NewForConfig(c.config)
if err != nil {
return errors.Wrap(err, "failed to get a k8s dynamic client")
}

c.crClient = crClient
c.clientset = clientset
c.dynClient = dynClient
c.recorder = eventer.NewEventRecorder(c.clientset, "Kanister Controller")

for cr, o := range map[customresource.CustomResource]runtime.Object{
Expand Down Expand Up @@ -360,7 +368,7 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
if err != nil {
return errors.WithStack(err)
}
tp, err := param.New(ctx, c.clientset, c.crClient, action)
tp, err := param.New(ctx, c.clientset, c.dynClient, c.crClient, action)
if err != nil {
return err
}
Expand Down
8 changes: 5 additions & 3 deletions pkg/function/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ import (
"fmt"

. "gopkg.in/check.v1"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes"
k8sscheme "k8s.io/client-go/kubernetes/scheme"

kanister "github.com/kanisterio/kanister/pkg"
crv1alpha1 "github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1"
Expand Down Expand Up @@ -296,7 +298,7 @@ func (s *DataSuite) getTemplateParamsAndPVCName(c *C, replicas int32) (*param.Te
},
}

tp, err := param.New(ctx, s.cli, s.crCli, as)
tp, err := param.New(ctx, s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, ss), s.crCli, as)
c.Assert(err, IsNil)
tp.Profile = s.profile

Expand Down Expand Up @@ -528,7 +530,7 @@ func (s *DataSuite) initPVCTemplateParams(c *C, pvc *v1.PersistentVolumeClaim, o
},
Options: options,
}
tp, err := param.New(context.Background(), s.cli, s.crCli, as)
tp, err := param.New(context.Background(), s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, pvc), s.crCli, as)
c.Assert(err, IsNil)
tp.Profile = s.profile
return tp
Expand Down
4 changes: 3 additions & 1 deletion pkg/function/e2e_volume_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import (
"k8s.io/api/core/v1"
k8sresource "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes"
k8sscheme "k8s.io/client-go/kubernetes/scheme"

kanister "github.com/kanisterio/kanister/pkg"
crv1alpha1 "github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1"
Expand Down Expand Up @@ -113,7 +115,7 @@ func (s *VolumeSnapshotTestSuite) SetUpTest(c *C) {
},
}

tp, err := param.New(ctx, s.cli, s.crCli, as)
tp, err := param.New(ctx, s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, ss), s.crCli, as)
c.Assert(err, IsNil)
s.tp = tp

Expand Down
6 changes: 4 additions & 2 deletions pkg/function/kube_exec_all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import (
. "gopkg.in/check.v1"
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes"
k8sscheme "k8s.io/client-go/kubernetes/scheme"

kanister "github.com/kanisterio/kanister/pkg"
crv1alpha1 "github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1"
Expand Down Expand Up @@ -122,7 +124,7 @@ func (s *KubeExecAllTest) TestKubeExecAllDeployment(c *C) {
Namespace: s.namespace,
},
}
tp, err := param.New(ctx, s.cli, s.crCli, as)
tp, err := param.New(ctx, s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, d), s.crCli, as)
c.Assert(err, IsNil)

action := "echo"
Expand Down Expand Up @@ -156,7 +158,7 @@ func (s *KubeExecAllTest) TestKubeExecAllStatefulSet(c *C) {
Namespace: s.namespace,
},
}
tp, err := param.New(ctx, s.cli, s.crCli, as)
tp, err := param.New(ctx, s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, ss), s.crCli, as)
c.Assert(err, IsNil)

action := "echo"
Expand Down
6 changes: 4 additions & 2 deletions pkg/function/kube_exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ import (
"strings"

. "gopkg.in/check.v1"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes"
k8sscheme "k8s.io/client-go/kubernetes/scheme"

kanister "github.com/kanisterio/kanister/pkg"
crv1alpha1 "github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1"
Expand Down Expand Up @@ -151,7 +153,7 @@ func (s *KubeExecTest) TestKubeExec(c *C) {
Namespace: s.namespace,
},
}
tp, err := param.New(ctx, s.cli, s.crCli, as)
tp, err := param.New(ctx, s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, ss), s.crCli, as)
c.Assert(err, IsNil)

action := "echo"
Expand Down
6 changes: 4 additions & 2 deletions pkg/function/scale_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import (
. "gopkg.in/check.v1"
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes"
k8sscheme "k8s.io/client-go/kubernetes/scheme"

kanister "github.com/kanisterio/kanister/pkg"
crv1alpha1 "github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1"
Expand Down Expand Up @@ -154,7 +156,7 @@ func (s *ScaleSuite) TestScaleDeployment(c *C) {
},
}
for _, action := range []string{"scaleUp", "echoHello", "scaleDown"} {
tp, err := param.New(ctx, s.cli, s.crCli, as)
tp, err := param.New(ctx, s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, d), s.crCli, as)
c.Assert(err, IsNil)
bp := newScaleBlueprint(kind)
phases, err := kanister.GetPhases(*bp, action, *tp)
Expand Down Expand Up @@ -203,7 +205,7 @@ func (s *ScaleSuite) TestScaleStatefulSet(c *C) {
}

for _, action := range []string{"scaleUp", "echoHello", "scaleDown"} {
tp, err := param.New(ctx, s.cli, s.crCli, as)
tp, err := param.New(ctx, s.cli, fake.NewSimpleDynamicClient(k8sscheme.Scheme, ss), s.crCli, as)
c.Assert(err, IsNil)
bp := newScaleBlueprint(kind)
phases, err := kanister.GetPhases(*bp, action, *tp)
Expand Down
21 changes: 15 additions & 6 deletions pkg/kube/unstructured.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,15 @@ func FetchUnstructuredObject(resource schema.GroupVersionResource, namespace, na
if err != nil {
return nil, err
}
return fetchCR(cli, resource, namespace, name)
return FetchUnstructuredObjectWithCli(cli, resource, namespace, name)
}

func fetchCR(cli dynamic.Interface, resource schema.GroupVersionResource, namespace, name string) (runtime.Unstructured, error) {
// FetchUnstructuredObjectWithCli returns the referenced API object as a map[string]interface{} using the specified CLI
// TODO: deprecate `FetchUnstructuredObject`
func FetchUnstructuredObjectWithCli(cli dynamic.Interface, resource schema.GroupVersionResource, namespace, name string) (runtime.Unstructured, error) {
if namespace == "" {
cli.Resource(resource).Get(name, metav1.GetOptions{})
}
return cli.Resource(resource).Namespace(namespace).Get(name, metav1.GetOptions{})
}

Expand All @@ -40,12 +45,16 @@ func ListUnstructuredObject(resource schema.GroupVersionResource, namespace stri
if err != nil {
return nil, err
}
return listCR(cli, resource, namespace)
return ListUnstructuredObjectWithCli(cli, resource, namespace)
}

func listCR(cli dynamic.Interface, resource schema.GroupVersionResource, namespace string) (runtime.Unstructured, error) {
//return cli.Resource(resource).Namespace(namespace).List(metav1.ListOptions{})
return cli.Resource(resource).List(metav1.ListOptions{})
// ListUnstructuredObjectWithCli returns the referenced API objects as a map[string]interface{} using the specified CLI
// TODO: deprecate `ListUnstructuredObject`
func ListUnstructuredObjectWithCli(cli dynamic.Interface, resource schema.GroupVersionResource, namespace string) (runtime.Unstructured, error) {
if namespace == "" {
return cli.Resource(resource).List(metav1.ListOptions{})
}
return cli.Resource(resource).Namespace(namespace).List(metav1.ListOptions{})
}

func client() (dynamic.Interface, error) {
Expand Down
27 changes: 18 additions & 9 deletions pkg/param/param.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ import (
"time"

"github.com/pkg/errors"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"

crv1alpha1 "github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1"
Expand Down Expand Up @@ -118,7 +119,7 @@ const (
)

// New function fetches and returns the desired params
func New(ctx context.Context, cli kubernetes.Interface, crCli versioned.Interface, as crv1alpha1.ActionSpec) (*TemplateParams, error) {
func New(ctx context.Context, cli kubernetes.Interface, dynCli dynamic.Interface, crCli versioned.Interface, as crv1alpha1.ActionSpec) (*TemplateParams, error) {
secrets, err := fetchSecrets(ctx, cli, as.Secrets)
if err != nil {
return nil, err
Expand All @@ -140,40 +141,48 @@ func New(ctx context.Context, cli kubernetes.Interface, crCli versioned.Interfac
Time: now.Format(timeFormat),
Options: as.Options,
}
var gvr schema.GroupVersionResource
namespace := as.Object.Namespace
switch strings.ToLower(as.Object.Kind) {
case StatefulSetKind:
ssp, err := fetchStatefulSetParams(ctx, cli, as.Object.Namespace, as.Object.Name)
if err != nil {
return nil, err
}
tp.StatefulSet = ssp
gvr = schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "statefulsets"}
case DeploymentKind:
dp, err := fetchDeploymentParams(ctx, cli, as.Object.Namespace, as.Object.Name)
if err != nil {
return nil, err
}
tp.Deployment = dp
gvr = schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}
case PVCKind:
pp, err := fetchPVCParams(ctx, cli, as.Object.Namespace, as.Object.Name)
if err != nil {
return nil, err
}
tp.PVC = pp
gvr = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "persistentvolumeclaims"}
case NamespaceKind:
tp.Namespace = &NamespaceParams{Name: as.Object.Namespace}
gvr = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}
// `Namespace` is a global resource
namespace = ""
default:
gvr := schema.GroupVersionResource{
gvr = schema.GroupVersionResource{
Group: as.Object.Group,
Version: as.Object.APIVersion,
Resource: as.Object.Resource,
}
u, err := kube.FetchUnstructuredObject(gvr, as.Object.Namespace, as.Object.Name)
if err != nil {
return nil, errors.Wrapf(err, "could not fetch object name: %s, namespace: %s, group: %s, version: %s, resource: %s", as.Object.Name, as.Object.Namespace, gvr.Group, gvr.Version, gvr.Resource)
}
// TODO: We should set `Object` for all other kinds as well.
tp.Object = u.UnstructuredContent()
}
u, err := kube.FetchUnstructuredObjectWithCli(dynCli, gvr, namespace, as.Object.Name)
if err != nil {
return nil, errors.Wrapf(err, "could not fetch object name: %s, namespace: %s, group: %s, version: %s, resource: %s", as.Object.Name, namespace, gvr.Group, gvr.Version, gvr.Resource)
}
tp.Object = u.UnstructuredContent()

return &tp, nil
}

Expand Down
Loading

0 comments on commit da43bcd

Please sign in to comment.