diff --git a/integration/util.go b/integration/util.go index e7a04f66d09..2d02a2e0655 100644 --- a/integration/util.go +++ b/integration/util.go @@ -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" @@ -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) } diff --git a/pkg/skaffold/build/cluster/kaniko.go b/pkg/skaffold/build/cluster/kaniko.go index b09431797ca..658d5250f31 100644 --- a/pkg/skaffold/build/cluster/kaniko.go +++ b/pkg/skaffold/build/cluster/kaniko.go @@ -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 { diff --git a/pkg/skaffold/build/cluster/secret.go b/pkg/skaffold/build/cluster/secret.go index d71193d0879..626cae3fc59 100644 --- a/pkg/skaffold/build/cluster/secret.go +++ b/pkg/skaffold/build/cluster/secret.go @@ -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") } @@ -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") } diff --git a/pkg/skaffold/build/cluster/sources/localdir.go b/pkg/skaffold/build/cluster/sources/localdir.go index 16f547ccdde..1d279ac8b80 100644 --- a/pkg/skaffold/build/cluster/sources/localdir.go +++ b/pkg/skaffold/build/cluster/sources/localdir.go @@ -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") } diff --git a/pkg/skaffold/deploy/labels.go b/pkg/skaffold/deploy/labels.go index c76ed41ed87..9e7058acc85 100644 --- a/pkg/skaffold/deploy/labels.go +++ b/pkg/skaffold/deploy/labels.go @@ -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 } diff --git a/pkg/skaffold/deploy/status_check.go b/pkg/skaffold/deploy/status_check.go index 3bc29e5eef5..9d23cc4be09 100644 --- a/pkg/skaffold/deploy/status_check.go +++ b/pkg/skaffold/deploy/status_check.go @@ -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" ) @@ -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) diff --git a/pkg/skaffold/kubernetes/client.go b/pkg/skaffold/kubernetes/client.go index 1a6aef29805..a93b532a329 100644 --- a/pkg/skaffold/kubernetes/client.go +++ b/pkg/skaffold/kubernetes/client.go @@ -27,7 +27,13 @@ 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") @@ -35,7 +41,7 @@ func GetClientset() (kubernetes.Interface, error) { 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") diff --git a/pkg/skaffold/kubernetes/log.go b/pkg/skaffold/kubernetes/log.go index e5cfe03958a..a0d1e5130fc 100644 --- a/pkg/skaffold/kubernetes/log.go +++ b/pkg/skaffold/kubernetes/log.go @@ -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 diff --git a/pkg/skaffold/kubernetes/owner.go b/pkg/skaffold/kubernetes/owner.go index d4b5e449afa..60fc7866e3f 100644 --- a/pkg/skaffold/kubernetes/owner.go +++ b/pkg/skaffold/kubernetes/owner.go @@ -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 { @@ -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 } diff --git a/pkg/skaffold/kubernetes/owner_test.go b/pkg/skaffold/kubernetes/owner_test.go index 6f9a00283a3..86cc66142b9 100644 --- a/pkg/skaffold/kubernetes/owner_test.go +++ b/pkg/skaffold/kubernetes/owner_test.go @@ -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" ) @@ -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) }) } @@ -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) }) diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder.go index 608d08296aa..5cf8ffae740 100644 --- a/pkg/skaffold/kubernetes/portforward/resource_forwarder.go +++ b/pkg/skaffold/kubernetes/portforward/resource_forwarder.go @@ -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 @@ -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 { diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go index 0cb138eee13..5da3d8929d9 100644 --- a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go +++ b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go @@ -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" ) @@ -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) }) diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go index 5b77c2abf32..8a726425547 100644 --- a/pkg/skaffold/sync/sync.go +++ b/pkg/skaffold/sync/sync.go @@ -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 diff --git a/pkg/webhook/kubernetes/deployment.go b/pkg/webhook/kubernetes/deployment.go index e3c1318d50c..51c058fc7e5 100644 --- a/pkg/webhook/kubernetes/deployment.go +++ b/pkg/webhook/kubernetes/deployment.go @@ -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 @@ -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) diff --git a/pkg/webhook/kubernetes/service.go b/pkg/webhook/kubernetes/service.go index a7a54f2062d..f402bdec71a 100644 --- a/pkg/webhook/kubernetes/service.go +++ b/pkg/webhook/kubernetes/service.go @@ -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} @@ -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 @@ -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{}) }