From b23c2158e72093fb66925c4225e50f1b796b4c12 Mon Sep 17 00:00:00 2001 From: balopat Date: Thu, 3 Oct 2019 21:29:22 -0700 Subject: [PATCH 1/3] fix deploy logging --- cmd/skaffold/app/cmd/deploy.go | 2 +- pkg/skaffold/kubernetes/log.go | 2 ++ pkg/skaffold/kubernetes/watcher.go | 2 ++ pkg/skaffold/runner/build_deploy.go | 3 ++- pkg/skaffold/runner/build_deploy_test.go | 32 ++++++++++++++++++++++++ pkg/skaffold/runner/runner_test.go | 8 ++++++ 6 files changed, 47 insertions(+), 2 deletions(-) diff --git a/cmd/skaffold/app/cmd/deploy.go b/cmd/skaffold/app/cmd/deploy.go index 0066ae6a21a..47afa0491e6 100644 --- a/cmd/skaffold/app/cmd/deploy.go +++ b/cmd/skaffold/app/cmd/deploy.go @@ -43,7 +43,7 @@ func NewCmdDeploy() *cobra.Command { WithFlags(func(f *pflag.FlagSet) { f.VarP(&preBuiltImages, "images", "i", "A list of pre-built images to deploy") f.VarP(&buildOutputFile, "build-artifacts", "a", `Filepath containing build output. -E.g. build.out created by running skaffold build --quiet -o "{{json .}}" > build.out`) +E.g. build.out created by running skaffold build --quiet -o {{json .}} > build.out`) }). NoArgs(cancelWithCtrlC(context.Background(), doDeploy)) } diff --git a/pkg/skaffold/kubernetes/log.go b/pkg/skaffold/kubernetes/log.go index a0d1e5130fc..ce61b2f53f8 100644 --- a/pkg/skaffold/kubernetes/log.go +++ b/pkg/skaffold/kubernetes/log.go @@ -88,6 +88,7 @@ func (a *LogAggregator) Start(ctx context.Context) error { case <-cancelCtx.Done(): return case evt, ok := <-aggregate: + if !ok { return } @@ -96,6 +97,7 @@ func (a *LogAggregator) Start(ctx context.Context) error { if !ok { continue } + logrus.Tracef("logger saw a pod event: %s - %s - watch: %t \n\n", pod.Name, pod.Status.Phase, a.podSelector.Select(pod)) if !a.podSelector.Select(pod) { continue diff --git a/pkg/skaffold/kubernetes/watcher.go b/pkg/skaffold/kubernetes/watcher.go index ce3eb1988f3..2bd3046bc65 100644 --- a/pkg/skaffold/kubernetes/watcher.go +++ b/pkg/skaffold/kubernetes/watcher.go @@ -18,6 +18,7 @@ package kubernetes import ( "github.com/pkg/errors" + "github.com/sirupsen/logrus" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/watch" ) @@ -39,6 +40,7 @@ func AggregatePodWatcher(namespaces []string, aggregate chan<- watch.Event) (fun var forever int64 = 3600 * 24 * 365 * 100 for _, ns := range namespaces { + logrus.Tracef("starting pod watcher in namespace '%s'", ns) watcher, err := kubeclient.CoreV1().Pods(ns).Watch(meta_v1.ListOptions{ TimeoutSeconds: &forever, }) diff --git a/pkg/skaffold/runner/build_deploy.go b/pkg/skaffold/runner/build_deploy.go index 4e609af2a72..9fe330699dd 100644 --- a/pkg/skaffold/runner/build_deploy.go +++ b/pkg/skaffold/runner/build_deploy.go @@ -60,7 +60,7 @@ func (r *SkaffoldRunner) BuildAndTest(ctx context.Context, out io.Writer, artifa return nil, err } - // Update which images are logged. + // Update which images are logged and portforwarded. for _, build := range bRes { r.podSelector.Add(build.Tag) } @@ -92,6 +92,7 @@ func (r *SkaffoldRunner) DeployAndLog(ctx context.Context, out io.Writer, artifa var imageNames []string for _, artifact := range artifacts { imageNames = append(imageNames, artifact.ImageName) + r.podSelector.Add(artifact.Tag) } logger := r.newLoggerForImages(out, imageNames) diff --git a/pkg/skaffold/runner/build_deploy_test.go b/pkg/skaffold/runner/build_deploy_test.go index 79383686a79..1a1a93d3e9e 100644 --- a/pkg/skaffold/runner/build_deploy_test.go +++ b/pkg/skaffold/runner/build_deploy_test.go @@ -21,12 +21,44 @@ import ( "io/ioutil" "testing" + v1 "k8s.io/api/core/v1" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/testutil" "github.com/pkg/errors" "k8s.io/client-go/tools/clientcmd/api" ) +func TestDeployLogging(t *testing.T) { + testutil.Run(t, "should list pods for logging", func(t *testutil.T) { + t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"}) + + ctx := context.Background() + artifacts := []build.Artifact{{ + ImageName: "img", + Tag: "img:1", + }} + + bench := &TestBench{} + runner := createRunner(t, bench, nil) + runner.runCtx.Opts.Tail = true + + err := runner.DeployAndLog(ctx, ioutil.Discard, artifacts) + + t.CheckErrorAndDeepEqual(false, err, []Actions{{Deployed: []string{"img:1"}}}, bench.Actions()) + t.CheckDeepEqual(true, runner.podSelector.Select(&v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: "img:1", + }, + }, + }, + })) + }) +} func TestBuildTestDeploy(t *testing.T) { tests := []struct { description string diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index c8a21b4adf0..abc484a49a3 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -23,6 +23,8 @@ import ( "io/ioutil" "testing" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/local" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" @@ -108,6 +110,12 @@ func (t *TestBench) enterNewCycle() { t.currentActions = Actions{} } +func (t *TestBench) newLoggerForImages(out io.Writer, images []string) *kubernetes.LogAggregator { + t.actions = append(t.actions, t.currentActions) + t.currentActions = Actions{} + return &kubernetes.LogAggregator{} +} + func (t *TestBench) Build(_ context.Context, _ io.Writer, _ tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) { if len(t.buildErrors) > 0 { err := t.buildErrors[0] From f405b25cdd03b503fbf2a6d878d4c2c3c786302a Mon Sep 17 00:00:00 2001 From: balopat Date: Fri, 4 Oct 2019 10:28:18 -0700 Subject: [PATCH 2/3] remove unused function --- pkg/skaffold/runner/runner_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index abc484a49a3..c8a21b4adf0 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -23,8 +23,6 @@ import ( "io/ioutil" "testing" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/local" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" @@ -110,12 +108,6 @@ func (t *TestBench) enterNewCycle() { t.currentActions = Actions{} } -func (t *TestBench) newLoggerForImages(out io.Writer, images []string) *kubernetes.LogAggregator { - t.actions = append(t.actions, t.currentActions) - t.currentActions = Actions{} - return &kubernetes.LogAggregator{} -} - func (t *TestBench) Build(_ context.Context, _ io.Writer, _ tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) { if len(t.buildErrors) > 0 { err := t.buildErrors[0] From 48ff7f16d7140b85c99f97efa5e65e5cdf38c73b Mon Sep 17 00:00:00 2001 From: balopat Date: Thu, 10 Oct 2019 15:22:49 -0700 Subject: [PATCH 3/3] accidental change --- cmd/skaffold/app/cmd/deploy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/skaffold/app/cmd/deploy.go b/cmd/skaffold/app/cmd/deploy.go index 47afa0491e6..0066ae6a21a 100644 --- a/cmd/skaffold/app/cmd/deploy.go +++ b/cmd/skaffold/app/cmd/deploy.go @@ -43,7 +43,7 @@ func NewCmdDeploy() *cobra.Command { WithFlags(func(f *pflag.FlagSet) { f.VarP(&preBuiltImages, "images", "i", "A list of pre-built images to deploy") f.VarP(&buildOutputFile, "build-artifacts", "a", `Filepath containing build output. -E.g. build.out created by running skaffold build --quiet -o {{json .}} > build.out`) +E.g. build.out created by running skaffold build --quiet -o "{{json .}}" > build.out`) }). NoArgs(cancelWithCtrlC(context.Background(), doDeploy)) }