Skip to content

Commit 5cd98ac

Browse files
JGAntunesJGAntunes
authored
fix(cluster_resources): pod disruption budgets for policy v1 not being collected (#1843)
* fix(cluster_resources): pod disruption budgets for policy v1 not being collected * fix: e2e test * fix: now actually fix the e2e test
1 parent 45e3e3a commit 5cd98ac

File tree

3 files changed

+181
-7
lines changed

3 files changed

+181
-7
lines changed

‎pkg/collect/cluster_resources.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -512,8 +512,8 @@ func pods(ctx context.Context, client *kubernetes.Clientset, namespaces []string
512512
return podsByNamespace, errorsByNamespace, unhealthyPods
513513
}
514514

515-
func getPodDisruptionBudgets(ctx context.Context, client *kubernetes.Clientset, namespaces []string) (map[string][]byte, map[string]string) {
516-
ok, err := discovery.HasResource(client, "policy/v1", "PodDisruptionBudgets")
515+
func getPodDisruptionBudgets(ctx context.Context, client kubernetes.Interface, namespaces []string) (map[string][]byte, map[string]string) {
516+
ok, err := discovery.HasResource(client.Discovery(), "policy/v1", "PodDisruptionBudget")
517517
if err != nil {
518518
return nil, map[string]string{"": err.Error()}
519519
}
@@ -525,7 +525,7 @@ func getPodDisruptionBudgets(ctx context.Context, client *kubernetes.Clientset,
525525
}
526526

527527
// TODO: The below function (`pdbV1`) needs to be DRY'd and moved into the main `getPodDisruptionBudgets` function.
528-
func pdbV1(ctx context.Context, client *kubernetes.Clientset, namespaces []string) (map[string][]byte, map[string]string) {
528+
func pdbV1(ctx context.Context, client kubernetes.Interface, namespaces []string) (map[string][]byte, map[string]string) {
529529
pdbByNamespace := make(map[string][]byte)
530530
errorsByNamespace := make(map[string]string)
531531

@@ -561,7 +561,7 @@ func pdbV1(ctx context.Context, client *kubernetes.Clientset, namespaces []strin
561561
}
562562

563563
// This block/function can remain as is
564-
func pdbV1beta(ctx context.Context, client *kubernetes.Clientset, namespaces []string) (map[string][]byte, map[string]string) {
564+
func pdbV1beta(ctx context.Context, client kubernetes.Interface, namespaces []string) (map[string][]byte, map[string]string) {
565565
pdbByNamespace := make(map[string][]byte)
566566
errorsByNamespace := make(map[string]string)
567567

‎pkg/collect/cluster_resources_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@ import (
1313
"github.com/stretchr/testify/require"
1414
v1 "k8s.io/api/coordination/v1"
1515
corev1 "k8s.io/api/core/v1"
16+
policyv1 "k8s.io/api/policy/v1"
17+
policyv1beta1 "k8s.io/api/policy/v1beta1"
1618
storagev1 "k8s.io/api/storage/v1"
1719
apixfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
1820
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21+
"k8s.io/apimachinery/pkg/util/intstr"
22+
fakediscovery "k8s.io/client-go/discovery/fake"
1923
testdynamicclient "k8s.io/client-go/dynamic/fake"
2024
"k8s.io/client-go/kubernetes"
2125
testclient "k8s.io/client-go/kubernetes/fake"
@@ -523,3 +527,173 @@ func fromJSON(t *testing.T, dat []byte) troubleshootv1beta2.SupportBundle {
523527
require.Equal(t, 1, len(sb))
524528
return sb[0]
525529
}
530+
531+
func Test_getPodDisruptionBudgets(t *testing.T) {
532+
tests := []struct {
533+
name string
534+
pdbNames []string
535+
namespaces []string
536+
}{
537+
{
538+
name: "single namespace",
539+
pdbNames: []string{"test-pdb"},
540+
namespaces: []string{"default"},
541+
},
542+
{
543+
name: "multiple namespaces",
544+
pdbNames: []string{"test-pdb"},
545+
namespaces: []string{"default", "test"},
546+
},
547+
{
548+
name: "multiple pdbs in different namespaces",
549+
pdbNames: []string{"test-pdb", "another-pdb"},
550+
namespaces: []string{"default", "test"},
551+
},
552+
}
553+
for _, tt := range tests {
554+
t.Run(tt.name, func(t *testing.T) {
555+
client := testclient.NewClientset()
556+
ctx := context.Background()
557+
err := createTestPodDisruptionBudgets(client, tt.pdbNames, tt.namespaces)
558+
assert.NoError(t, err)
559+
560+
fakeDiscovery, ok := client.Discovery().(*fakediscovery.FakeDiscovery)
561+
if !ok {
562+
t.Fatalf("could not convert Discovery() to *FakeDiscovery")
563+
}
564+
fakeDiscovery.Resources = []*metav1.APIResourceList{
565+
{
566+
GroupVersion: "policy/v1",
567+
APIResources: []metav1.APIResource{
568+
{
569+
Kind: "PodDisruptionBudget",
570+
},
571+
},
572+
},
573+
}
574+
575+
pdbs, errors := getPodDisruptionBudgets(ctx, client, tt.namespaces)
576+
assert.Empty(t, errors)
577+
assert.Equal(t, len(tt.namespaces), len(pdbs))
578+
579+
for _, ns := range tt.namespaces {
580+
assert.NotEmpty(t, pdbs[ns+".json"])
581+
var pdbList policyv1.PodDisruptionBudgetList
582+
err := json.Unmarshal(pdbs[ns+".json"], &pdbList)
583+
assert.NoError(t, err)
584+
assert.Equal(t, len(tt.pdbNames), len(pdbList.Items))
585+
for _, pdb := range pdbList.Items {
586+
assert.Contains(t, tt.pdbNames, pdb.ObjectMeta.Name)
587+
}
588+
}
589+
})
590+
}
591+
}
592+
593+
func Test_getPodDisruptionBudgets_v1beta1(t *testing.T) {
594+
tests := []struct {
595+
name string
596+
pdbNames []string
597+
namespaces []string
598+
}{
599+
{
600+
name: "single namespace v1beta1",
601+
pdbNames: []string{"test-pdb-beta"},
602+
namespaces: []string{"default"},
603+
},
604+
{
605+
name: "multiple namespaces v1beta1",
606+
pdbNames: []string{"test-pdb-beta"},
607+
namespaces: []string{"default", "test"},
608+
},
609+
}
610+
for _, tt := range tests {
611+
t.Run(tt.name, func(t *testing.T) {
612+
client := testclient.NewClientset()
613+
ctx := context.Background()
614+
err := createTestPodDisruptionBudgetsV1beta1(client, tt.pdbNames, tt.namespaces)
615+
assert.NoError(t, err)
616+
617+
fakeDiscovery, ok := client.Discovery().(*fakediscovery.FakeDiscovery)
618+
if !ok {
619+
t.Fatalf("could not convert Discovery() to *FakeDiscovery")
620+
}
621+
// Mock discovery to only have v1beta1 PodDisruptionBudget
622+
fakeDiscovery.Resources = []*metav1.APIResourceList{
623+
{
624+
GroupVersion: "policy/v1beta1",
625+
APIResources: []metav1.APIResource{
626+
{
627+
Kind: "PodDisruptionBudget",
628+
},
629+
},
630+
},
631+
}
632+
633+
pdbs, errors := getPodDisruptionBudgets(ctx, client, tt.namespaces)
634+
assert.Empty(t, errors)
635+
assert.Equal(t, len(tt.namespaces), len(pdbs))
636+
637+
for _, ns := range tt.namespaces {
638+
assert.NotEmpty(t, pdbs[ns+".json"])
639+
var pdbList policyv1beta1.PodDisruptionBudgetList
640+
err := json.Unmarshal(pdbs[ns+".json"], &pdbList)
641+
assert.NoError(t, err)
642+
assert.Equal(t, len(tt.pdbNames), len(pdbList.Items))
643+
for _, pdb := range pdbList.Items {
644+
assert.Contains(t, tt.pdbNames, pdb.ObjectMeta.Name)
645+
}
646+
}
647+
})
648+
}
649+
}
650+
651+
func createTestPodDisruptionBudgets(client kubernetes.Interface, pdbNames []string, namespaces []string) error {
652+
for _, ns := range namespaces {
653+
for _, pdbName := range pdbNames {
654+
minAvailable := intstr.FromInt32(1)
655+
_, err := client.PolicyV1().PodDisruptionBudgets(ns).Create(context.Background(), &policyv1.PodDisruptionBudget{
656+
ObjectMeta: metav1.ObjectMeta{
657+
Name: pdbName,
658+
},
659+
Spec: policyv1.PodDisruptionBudgetSpec{
660+
MinAvailable: &minAvailable,
661+
Selector: &metav1.LabelSelector{
662+
MatchLabels: map[string]string{
663+
"app": "test-app",
664+
},
665+
},
666+
},
667+
}, metav1.CreateOptions{})
668+
if err != nil {
669+
return err
670+
}
671+
}
672+
}
673+
return nil
674+
}
675+
676+
func createTestPodDisruptionBudgetsV1beta1(client kubernetes.Interface, pdbNames []string, namespaces []string) error {
677+
for _, ns := range namespaces {
678+
for _, pdbName := range pdbNames {
679+
minAvailable := intstr.FromInt32(1)
680+
_, err := client.PolicyV1beta1().PodDisruptionBudgets(ns).Create(context.Background(), &policyv1beta1.PodDisruptionBudget{
681+
ObjectMeta: metav1.ObjectMeta{
682+
Name: pdbName,
683+
},
684+
Spec: policyv1beta1.PodDisruptionBudgetSpec{
685+
MinAvailable: &minAvailable,
686+
Selector: &metav1.LabelSelector{
687+
MatchLabels: map[string]string{
688+
"app": "test-app",
689+
},
690+
},
691+
},
692+
}, metav1.CreateOptions{})
693+
if err != nil {
694+
return err
695+
}
696+
}
697+
}
698+
return nil
699+
}

‎test/e2e/support-bundle/cluster_resources_e2e_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ func TestClusterResources(t *testing.T) {
2424
"volumeattachments.json",
2525
"nodes.json",
2626
"pvs.json",
27-
"pod-disruption-budgets-errors.json",
2827
"resources.json",
2928
"custom-resource-definitions.json",
3029
"groups.json",
@@ -58,13 +57,14 @@ func TestClusterResources(t *testing.T) {
5857
"resource-quota",
5958
"ingress",
6059
"pods",
60+
"pod-disruption-budgets",
6161
},
6262
expectType: "folder",
6363
},
6464
}
6565

66-
feature := features.New("Cluster Resouces Test").
67-
Assess("check support bundle catch cluster resouces", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
66+
feature := features.New("Cluster Resources Test").
67+
Assess("check support bundle catch cluster resources", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
6868
var out bytes.Buffer
6969
supportBundleName := "cluster-resources"
7070
tarPath := fmt.Sprintf("%s.tar.gz", supportBundleName)

0 commit comments

Comments
 (0)