Skip to content

Commit

Permalink
fix: Fix standalone pods and deployment health check pending. (#7691)
Browse files Browse the repository at this point in the history
* remove empty namespace check in diag.NeW

* remove debug statements

* fix tests and lint

* fix lint
  • Loading branch information
tejal29 authored Jul 28, 2022
1 parent 3bedcf3 commit fd7c4bd
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 9 deletions.
8 changes: 1 addition & 7 deletions pkg/diag/diag.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,8 @@ type diag struct {
}

func New(namespaces []string) Diagnose {
var ns []string
for _, n := range namespaces {
if n != "" {
ns = append(ns, n)
}
}
return &diag{
namespaces: ns,
namespaces: namespaces,
labels: map[string]string{},
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/diag/diag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ func TestRun(t *testing.T) {
description: "multiple namespaces with an empty namespace and no labels",
ns: []string{"foo", "bar", ""},
expected: &mockValidator{
ns: []string{"foo", "bar"},
ns: []string{"foo", "bar", ""},
listOptions: metav1.ListOptions{},
},
},
{
description: "empty namespaces no labels",
ns: []string{""},
expected: &mockValidator{ns: nil},
expected: &mockValidator{ns: []string{""}},
},
{
description: "multiple namespaces and multiple labels",
Expand Down
78 changes: 78 additions & 0 deletions pkg/skaffold/deploy/util/namespaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ package util
import (
"testing"

"k8s.io/client-go/tools/clientcmd/api"

kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
Expand Down Expand Up @@ -94,3 +97,78 @@ func TestCollectHelmReleasesNamespaces(t *testing.T) {
})
}
}

func TestGetAllPodNamespaces(t *testing.T) {
tests := []struct {
description string
ns string
helmReleases []latest.HelmRelease
apiConfig api.Config
env []string
expected []string
shouldErr bool
}{
{
description: "current config empty, ns empty with helm releases",
ns: "",
apiConfig: api.Config{CurrentContext: ""},
helmReleases: []latest.HelmRelease{
{
Namespace: "foo",
},
{
Namespace: "bar",
},
{
Namespace: "baz",
},
},
expected: []string{"", "bar", "baz", "foo"},
},
{
description: "current config empty, ns empty",
ns: "",
apiConfig: api.Config{CurrentContext: ""},
expected: []string{""},
},
{
description: "ns empty, current config set",
ns: "",
apiConfig: api.Config{CurrentContext: "test",
Contexts: map[string]*api.Context{
"test": {Namespace: "test-ns"}}},
expected: []string{"test-ns"},
},
{
description: "ns set and current config set",
ns: "cli-ns",
apiConfig: api.Config{CurrentContext: "test",
Contexts: map[string]*api.Context{
"test": {Namespace: "test-ns"}}},
expected: []string{"cli-ns"},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&util.OSEnviron, func() []string { return test.env })
t.Override(&kubectx.CurrentConfig, func() (api.Config, error) {
return test.apiConfig, nil
})
ns, err := GetAllPodNamespaces(test.ns, []latest.Pipeline{
{
Deploy: latest.DeployConfig{
DeployType: latest.DeployType{
LegacyHelmDeploy: &latest.LegacyHelmDeploy{
Releases: test.helmReleases,
},
},
},
},
})
t.CheckError(test.shouldErr, err)
if !test.shouldErr {
t.CheckDeepEqual(test.expected, ns)
}
})
}
}
6 changes: 6 additions & 0 deletions pkg/skaffold/deploy/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ func TestConsolidateNamespaces(t *testing.T) {
oldNamespaces: []string{},
newNamespaces: []string{""},
},
{
description: "update namespace when namespace is empty string new namespace is nil",
oldNamespaces: []string{""},
newNamespaces: []string{},
expected: []string{""},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
Expand Down

0 comments on commit fd7c4bd

Please sign in to comment.