Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single way of mocking Kubernetes client/dynamic client #2796

Merged
merged 1 commit into from
Sep 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions integration/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"testing"
"time"

kubernetesutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
pkgkubernetes "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
"github.com/sirupsen/logrus"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
Expand All @@ -46,7 +46,7 @@ func Run(t *testing.T, dir, command string, args ...string) {

// SetupNamespace creates a Kubernetes namespace to run a test.
func SetupNamespace(t *testing.T) (*v1.Namespace, *NSKubernetesClient, func()) {
client, err := kubernetesutil.GetClientset()
client, err := pkgkubernetes.Client()
if err != nil {
t.Fatalf("Test setup error: getting kubernetes client: %s", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/build/cluster/kaniko.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ func (b *Builder) runKanikoBuild(ctx context.Context, out io.Writer, artifact *l
}

// Create pod
client, err := kubernetes.GetClientset()
client, err := kubernetes.Client()
if err != nil {
return "", errors.Wrap(err, "")
return "", errors.Wrap(err, "getting kubernetes client")
}
pods := client.CoreV1().Pods(b.Namespace)

pods := client.CoreV1().Pods(b.Namespace)
podSpec := s.Pod(args)
pod, err := pods.Create(podSpec)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/build/cluster/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
func (b *Builder) setupPullSecret(out io.Writer) (func(), error) {
color.Default.Fprintf(out, "Creating kaniko secret [%s/%s]...\n", b.Namespace, b.PullSecretName)

client, err := kubernetes.GetClientset()
client, err := kubernetes.Client()
if err != nil {
return nil, errors.Wrap(err, "getting kubernetes client")
}
Expand Down Expand Up @@ -82,7 +82,7 @@ func (b *Builder) setupDockerConfigSecret(out io.Writer) (func(), error) {

color.Default.Fprintf(out, "Creating docker config secret [%s]...\n", b.DockerConfig.SecretName)

client, err := kubernetes.GetClientset()
client, err := kubernetes.Client()
if err != nil {
return nil, errors.Wrap(err, "getting kubernetes client")
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/skaffold/build/cluster/sources/localdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,11 @@ func (g *LocalDir) Pod(args []string) *v1.Pod {
// Via kubectl exec, we extract the tarball to the empty dir
// Then, via kubectl exec, create the /tmp/complete file via kubectl exec to complete the init container
func (g *LocalDir) ModifyPod(ctx context.Context, p *v1.Pod) error {
client, err := kubernetes.GetClientset()
client, err := kubernetes.Client()
if err != nil {
return errors.Wrap(err, "getting clientset")
return errors.Wrap(err, "getting kubernetes client")
}

if err := kubernetes.WaitForPodInitialized(ctx, client.CoreV1().Pods(p.Namespace), p.Name); err != nil {
return errors.Wrap(err, "waiting for pod to initialize")
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/deploy/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ func labelDeployResults(labels map[string]string, results []Artifact) {
// use the kubectl client to update all k8s objects with a skaffold watermark
dynClient, err := kubernetes.DynamicClient()
if err != nil {
logrus.Warnf("error retrieving kubernetes dynamic client: %s", err.Error())
logrus.Warnf("error getting kubernetes dynamic client: %s", err.Error())
return
}

client, err := kubernetes.GetClientset()
client, err := kubernetes.Client()
if err != nil {
logrus.Warnf("error retrieving kubernetes client: %s", err.Error())
logrus.Warnf("error getting kubernetes client: %s", err.Error())
return
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/skaffold/deploy/status_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"k8s.io/client-go/kubernetes"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
kubernetesutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
pkgkubernetes "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
)

Expand All @@ -44,10 +44,11 @@ var (
)

func StatusCheck(ctx context.Context, defaultLabeller *DefaultLabeller, runCtx *runcontext.RunContext) error {
client, err := kubernetesutil.GetClientset()
client, err := pkgkubernetes.Client()
if err != nil {
return err
return errors.Wrap(err, "getting kubernetes client")
}

deadline := getDeadline(runCtx.Cfg.Deploy.StatusCheckDeadlineSeconds)

dMap, err := getDeployments(client, runCtx.Opts.Namespace, defaultLabeller, deadline)
Expand Down
10 changes: 8 additions & 2 deletions pkg/skaffold/kubernetes/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,21 @@ import (
_ "k8s.io/client-go/plugin/pkg/client/auth"
)

func GetClientset() (kubernetes.Interface, error) {
// for tests
var (
Client = getClientset
DynamicClient = getDynamicClient
)

func getClientset() (kubernetes.Interface, error) {
config, err := context.GetRestClientConfig()
if err != nil {
return nil, errors.Wrap(err, "getting client config for kubernetes client")
}
return kubernetes.NewForConfig(config)
}

func GetDynamicClient() (dynamic.Interface, error) {
func getDynamicClient() (dynamic.Interface, error) {
config, err := context.GetRestClientConfig()
if err != nil {
return nil, errors.Wrap(err, "getting client config for dynamic client")
Expand Down
4 changes: 0 additions & 4 deletions pkg/skaffold/kubernetes/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
)

// Client is for tests
var Client = GetClientset
var DynamicClient = GetDynamicClient

// LogAggregator aggregates the logs for all the deployed pods.
type LogAggregator struct {
output io.Writer
Expand Down
7 changes: 1 addition & 6 deletions pkg/skaffold/kubernetes/owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var (
// For testing
getClientSet = GetClientset
)

// TopLevelOwnerKey returns a key associated with the top level
// owner of a Kubernetes resource in the form Kind-Name
func TopLevelOwnerKey(obj metav1.Object, kind string) string {
Expand All @@ -47,7 +42,7 @@ func TopLevelOwnerKey(obj metav1.Object, kind string) string {
}

func ownerMetaObject(ns string, owner metav1.OwnerReference) (metav1.Object, error) {
client, err := getClientSet()
client, err := Client()
if err != nil {
return nil, err
}
Expand Down
14 changes: 8 additions & 6 deletions pkg/skaffold/kubernetes/owner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@ import (

v1 "k8s.io/api/core/v1"

"github.com/GoogleContainerTools/skaffold/testutil"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
batchv1beta1 "k8s.io/api/batch/v1beta1"

appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"

"github.com/GoogleContainerTools/skaffold/testutil"
"k8s.io/client-go/kubernetes"
fakekubeclientset "k8s.io/client-go/kubernetes/fake"
)
Expand Down Expand Up @@ -103,8 +101,10 @@ func TestTopLevelOwnerKey(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
client := fakekubeclientset.NewSimpleClientset(test.objects...)
t.Override(&getClientSet, mockClient(client))
t.Override(&Client, mockClient(client))

actual := TopLevelOwnerKey(test.initialObject, test.kind)

t.CheckDeepEqual(test.expected, actual)
})
}
Expand Down Expand Up @@ -277,8 +277,10 @@ func TestOwnerMetaObject(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
client := fakekubeclientset.NewSimpleClientset(test.objects...)
t.Override(&getClientSet, mockClient(client))
t.Override(&Client, mockClient(client))

actual, err := ownerMetaObject("ns", test.or)

t.CheckNoError(err)
t.CheckDeepEqual(test.expected, actual)
})
Expand Down
7 changes: 3 additions & 4 deletions pkg/skaffold/kubernetes/portforward/resource_forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ var (
// For testing
retrieveAvailablePort = util.GetAvailablePort
retrieveServices = retrieveServiceResources
getClientSet = kubernetes.GetClientset
)

// NewResourceForwarder returns a struct that tracks and port-forwards pods as they are created and modified
Expand Down Expand Up @@ -101,14 +100,14 @@ func (p *ResourceForwarder) getCurrentEntry(resource latest.PortForwardResource)
// retrieveServiceResources retrieves all services in the cluster matching the given label
// as a list of PortForwardResources
func retrieveServiceResources(label string, namespaces []string) ([]*latest.PortForwardResource, error) {
clientset, err := getClientSet()
client, err := kubernetes.Client()
if err != nil {
return nil, errors.Wrap(err, "getting clientset")
return nil, errors.Wrap(err, "getting kubernetes client")
}

var resources []*latest.PortForwardResource
for _, ns := range namespaces {
services, err := clientset.CoreV1().Services(ns).List(metav1.ListOptions{
services, err := client.CoreV1().Services(ns).List(metav1.ListOptions{
LabelSelector: label,
})
if err != nil {
Expand Down
16 changes: 9 additions & 7 deletions pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,19 @@ import (
"testing"
"time"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
kubernetesutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
"github.com/google/go-cmp/cmp"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
fakekubeclientset "k8s.io/client-go/kubernetes/fake"
)

Expand Down Expand Up @@ -344,8 +344,10 @@ func TestRetrieveServices(t *testing.T) {
objs[i] = s
}
client := fakekubeclientset.NewSimpleClientset(objs...)
t.Override(&getClientSet, mockClient(client))
t.Override(&kubernetesutil.Client, mockClient(client))

actual, err := retrieveServiceResources(fmt.Sprintf("%s=9876-6789", deploy.RunIDLabel), test.namespaces)

t.CheckNoError(err)
t.CheckDeepEqual(test.expected, actual)
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func Perform(ctx context.Context, image string, files syncMap, cmdFn func(contex

client, err := kubernetes.Client()
if err != nil {
return errors.Wrap(err, "getting k8s client")
return errors.Wrap(err, "getting kubernetes client")
}

numSynced := 0
Expand Down
12 changes: 7 additions & 5 deletions pkg/webhook/kubernetes/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ const (
// 2. A container to run hugo server
// and one emptyDir volume to hold the git repository
func CreateDeployment(pr *github.PullRequestEvent, svc *v1.Service, externalIP string) (*appsv1.Deployment, error) {
clientset, err := pkgkubernetes.GetClientset()
client, err := pkgkubernetes.Client()
if err != nil {
return nil, errors.Wrap(err, "getting clientset")
return nil, errors.Wrap(err, "getting kubernetes client")
}

deploymentLabels := svc.Spec.Selector
Expand Down Expand Up @@ -119,18 +119,20 @@ func CreateDeployment(pr *github.PullRequestEvent, svc *v1.Service, externalIP s
},
},
}
return clientset.AppsV1().Deployments(constants.Namespace).Create(d)
return client.AppsV1().Deployments(constants.Namespace).Create(d)
}

// WaitForDeploymentToStabilize waits till the Deployment has stabilized
func WaitForDeploymentToStabilize(d *appsv1.Deployment, ip string) error {
client, err := pkgkubernetes.GetClientset()
client, err := pkgkubernetes.Client()
if err != nil {
return errors.Wrap(err, "getting clientset")
return errors.Wrap(err, "getting kubernetes client")
}

if err := pkgkubernetes.WaitForDeploymentToStabilize(context.Background(), client, d.Namespace, d.Name, 5*time.Minute); err != nil {
return errors.Wrap(err, "waiting for deployment to stabilize")
}

// wait up to five minutes for the URL to return a valid endpoint
url := BaseURL(ip)
log.Printf("Waiting up to 2 minutes for %s to return an OK response...", url)
Expand Down
15 changes: 8 additions & 7 deletions pkg/webhook/kubernetes/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ import (
// CreateService creates a service for the deployment to bind to
// and returns the external IP of the service
func CreateService(pr *github.PullRequestEvent) (*v1.Service, error) {
clientset, err := kubernetes.GetClientset()
client, err := kubernetes.Client()
if err != nil {
return nil, errors.Wrap(err, "getting clientset")
return nil, errors.Wrap(err, "getting kubernetes client")
}
l := labels.GenerateLabelsFromPR(pr.GetNumber())

l := labels.GenerateLabelsFromPR(pr.GetNumber())
key, val := labels.RetrieveLabel(pr.GetNumber())
selector := map[string]string{key: val}

Expand All @@ -57,7 +57,7 @@ func CreateService(pr *github.PullRequestEvent) (*v1.Service, error) {
Selector: selector,
},
}
return clientset.CoreV1().Services(constants.Namespace).Create(svc)
return client.CoreV1().Services(constants.Namespace).Create(svc)
}

// GetExternalIP polls the service until an external IP is available and returns it
Expand All @@ -82,9 +82,10 @@ func serviceName(prNumber int) string {
}

func getService(svc *v1.Service) (*v1.Service, error) {
clientset, err := kubernetes.GetClientset()
client, err := kubernetes.Client()
if err != nil {
return nil, errors.Wrap(err, "getting clientset")
return nil, errors.Wrap(err, "getting kubernetes client")
}
return clientset.CoreV1().Services(svc.Namespace).Get(svc.Name, metav1.GetOptions{})

return client.CoreV1().Services(svc.Namespace).Get(svc.Name, metav1.GetOptions{})
}