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

fix: Fix standalone pods and deployment health check pending. #7691

Merged
merged 4 commits into from
Jul 28, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
remove empty namespace check in diag.NeW
  • Loading branch information
tejal29 committed Jul 27, 2022
commit 0b47b522494d40a59495288ec92eeae300717a14
4 changes: 1 addition & 3 deletions pkg/diag/diag.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ type diag struct {
func New(namespaces []string) Diagnose {
var ns []string
for _, n := range namespaces {
if n != "" {
ns = append(ns, n)
}
ns = append(ns, n)
}
return &diag{
namespaces: ns,
Expand Down
5 changes: 2 additions & 3 deletions pkg/skaffold/deploy/util/namespaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,9 @@ func GetAllPodNamespaces(configNamespace string, pipelines []latest.Pipeline) ([

// Collate the slice of namespaces.
namespaces := make([]string, 0, len(nsMap))
for ns := range nsMap {
namespaces = append(namespaces, ns)
for k, _ := range nsMap {
namespaces = append(namespaces, k)
}

sort.Strings(namespaces)
return namespaces, nil
}
Expand Down
77 changes: 77 additions & 0 deletions pkg/skaffold/deploy/util/namespaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ package util
import (
"testing"

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"
"k8s.io/client-go/tools/clientcmd/api"
)

func TestCollectHelmReleasesNamespaces(t *testing.T) {
Expand Down Expand Up @@ -94,3 +96,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)
}
})
}
}
2 changes: 2 additions & 0 deletions pkg/skaffold/deploy/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ func MockK8sClient(string) (k8s.Interface, error) {
}

func ConsolidateNamespaces(original, new []string) []string {
fmt.Println("old len", len(original), len(new))
fmt.Println("old ", original, new)
if len(new) == 0 {
return original
}
Expand Down
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
1 change: 1 addition & 0 deletions pkg/skaffold/kubernetes/status/status_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ func (s *monitor) statusCheck(ctx context.Context, out io.Writer) (proto.StatusC
}
resources := make([]*resource.Resource, 0)
for _, n := range *s.namespaces {
fmt.Println("namespace ---", n)
newDeployments, err := getDeployments(ctx, client, n, s.labeller, getDeadline(s.deadlineSeconds))
if err != nil {
return proto.StatusCode_STATUSCHECK_DEPLOYMENT_FETCH_ERR, fmt.Errorf("could not fetch deployments: %w", err)
Expand Down