From 4e77dab88130ff7ab4bb225d25f20d5a90458ebe Mon Sep 17 00:00:00 2001 From: Alfredo Garo Date: Fri, 21 Jul 2023 22:39:35 -0400 Subject: [PATCH] Proper use of verbs Get and List in functions/methods names --- kubedog.go | 6 ++--- pkg/kube/kube.go | 14 ++++++------ pkg/kube/pod/pod.go | 24 ++++++++------------ pkg/kube/pod/pod_helper.go | 3 +-- pkg/kube/structured/structured.go | 8 +++---- pkg/kube/structured/structured_helper.go | 9 ++++---- pkg/kube/structured/structured_test.go | 6 ++--- pkg/kube/unstructured/unstructured.go | 2 +- pkg/kube/unstructured/unstructured_helper.go | 2 +- pkg/kube/unstructured/unstructured_test.go | 8 +++---- 10 files changed, 37 insertions(+), 45 deletions(-) diff --git a/kubedog.go b/kubedog.go index 101c75e..e5c65aa 100644 --- a/kubedog.go +++ b/kubedog.go @@ -58,8 +58,8 @@ func (kdt *Test) SetScenario(scenario *godog.ScenarioContext) { kdt.scenario.Step(`^(?:I )?verify InstanceGroups (?:are )?in "ready" state$`, kdt.KubeClientSet.VerifyInstanceGroups) //syntax-generation:title-1:Structured Resources //syntax-generation:title-2:Pods - kdt.scenario.Step(`^(?:I )?get (?:the )?pods in namespace ([^"]*)$`, kdt.KubeClientSet.Pods) - kdt.scenario.Step(`^(?:I )?get (?:the )?pods in namespace ([^"]*) with selector (\S+)$`, kdt.KubeClientSet.PodsWithSelector) + kdt.scenario.Step(`^(?:I )?get (?:the )?pods in namespace ([^"]*)$`, kdt.KubeClientSet.ListPods) + kdt.scenario.Step(`^(?:I )?get (?:the )?pods in namespace ([^"]*) with selector (\S+)$`, kdt.KubeClientSet.ListPodsWithSelector) kdt.scenario.Step(`^(?:the )?pods in namespace ([^"]*) with selector (\S+) have restart count less than (\d+)$`, kdt.KubeClientSet.PodsWithSelectorHaveRestartCountLessThan) kdt.scenario.Step(`^(some|all) pods in namespace (\S+) with selector (\S+) have "([^"]*)" in logs since ([^"]*) time$`, kdt.KubeClientSet.SomeOrAllPodsInNamespaceWithSelectorHaveStringInLogsSinceTime) kdt.scenario.Step(`^some pods in namespace (\S+) with selector (\S+) don't have "([^"]*)" in logs since ([^"]*) time$`, kdt.KubeClientSet.SomePodsInNamespaceWithSelectorDontHaveStringInLogsSinceTime) @@ -74,7 +74,7 @@ func (kdt *Test) SetScenario(scenario *godog.ScenarioContext) { kdt.scenario.Step(`^(?:the )?(deployment|hpa|horizontalpodautoscaler|service|pdb|poddisruptionbudget|sa|serviceaccount) ([^"]*) (is|is not) in namespace ([^"]*)$`, kdt.KubeClientSet.ResourceInNamespace) kdt.scenario.Step(`^(?:I )?scale (?:the )?deployment ([^"]*) in namespace ([^"]*) to (\d+)$`, kdt.KubeClientSet.ScaleDeployment) kdt.scenario.Step(`^(?:I )?validate Prometheus Statefulset ([^"]*) in namespace ([^"]*) has volumeClaimTemplates name ([^"]*)$`, kdt.KubeClientSet.ValidatePrometheusVolumeClaimTemplatesName) - kdt.scenario.Step(`^(?:I )?get (?:the )?nodes list$`, kdt.KubeClientSet.GetNodes) + kdt.scenario.Step(`^(?:I )?get (?:the )?nodes list$`, kdt.KubeClientSet.ListNodes) kdt.scenario.Step(`^(?:the )?daemonset ([^"]*) is running in namespace ([^"]*)$`, kdt.KubeClientSet.DaemonSetIsRunning) kdt.scenario.Step(`^(?:the )?deployment ([^"]*) is running in namespace ([^"]*)$`, kdt.KubeClientSet.DeploymentIsRunning) kdt.scenario.Step(`^(?:the )?persistentvolume ([^"]*) exists with status (Available|Bound|Released|Failed|Pending)$`, kdt.KubeClientSet.PersistentVolExists) diff --git a/pkg/kube/kube.go b/pkg/kube/kube.go index 8928f8d..8d28431 100644 --- a/pkg/kube/kube.go +++ b/pkg/kube/kube.go @@ -104,7 +104,7 @@ func (kc *ClientSet) SetTimestamp(timestampName string) error { func (kc *ClientSet) KubernetesClusterShouldBe(state string) error { switch state { case common.StateCreated, common.StateUpgraded: - if err := pod.Pods(kc.KubeInterface, metav1.NamespaceSystem); err != nil { + if err := pod.ListPods(kc.KubeInterface, metav1.NamespaceSystem); err != nil { return errors.Errorf("failed validating cluster create/update, could not get pods: '%v'", err) } return nil @@ -209,12 +209,12 @@ func (kc *ClientSet) VerifyInstanceGroups() error { return unstruct.VerifyInstanceGroups(kc.DynamicInterface) } -func (kc *ClientSet) Pods(namespace string) error { - return pod.Pods(kc.KubeInterface, namespace) +func (kc *ClientSet) ListPods(namespace string) error { + return pod.ListPods(kc.KubeInterface, namespace) } -func (kc *ClientSet) PodsWithSelector(namespace, selector string) error { - return pod.PodsWithSelector(kc.KubeInterface, namespace, selector) +func (kc *ClientSet) ListPodsWithSelector(namespace, selector string) error { + return pod.ListPodsWithSelector(kc.KubeInterface, namespace, selector) } func (kc *ClientSet) PodsWithSelectorHaveRestartCountLessThan(namespace, selector string, restartCount int) error { @@ -292,8 +292,8 @@ func (kc *ClientSet) ValidatePrometheusVolumeClaimTemplatesName(statefulsetName, return structured.ValidatePrometheusVolumeClaimTemplatesName(kc.KubeInterface, statefulsetName, namespace, volumeClaimTemplatesName) } -func (kc *ClientSet) GetNodes() error { - return structured.GetNodes(kc.KubeInterface) +func (kc *ClientSet) ListNodes() error { + return structured.ListNodes(kc.KubeInterface) } func (kc *ClientSet) DaemonSetIsRunning(name, namespace string) error { diff --git a/pkg/kube/pod/pod.go b/pkg/kube/pod/pod.go index 64d5b9f..faaa342 100644 --- a/pkg/kube/pod/pod.go +++ b/pkg/kube/pod/pod.go @@ -30,17 +30,11 @@ import ( "k8s.io/client-go/kubernetes" ) -// TODO: look into functions name with verbs in names that do not match the behavior, -// pay attention to return object, some are function have verb List because they return a List, -// but that should be call GetList instead - -// TODO: should be ListPods because it prints them -func Pods(kubeClientset kubernetes.Interface, namespace string) error { - return PodsWithSelector(kubeClientset, namespace, "") +func ListPods(kubeClientset kubernetes.Interface, namespace string) error { + return ListPodsWithSelector(kubeClientset, namespace, "") } -// TODO: should be ListPodsWithSelector because it prints them -func PodsWithSelector(kubeClientset kubernetes.Interface, namespace, selector string) error { +func ListPodsWithSelector(kubeClientset kubernetes.Interface, namespace, selector string) error { var readyCountFn = func(conditions []corev1.ContainerStatus) string { var readyCount = 0 var containerCount = len(conditions) @@ -51,7 +45,7 @@ func PodsWithSelector(kubeClientset kubernetes.Interface, namespace, selector st } return fmt.Sprintf("%d/%d", readyCount, containerCount) } - pods, err := ListPodsWithLabelSelector(kubeClientset, namespace, selector) + pods, err := GetPodListWithLabelSelector(kubeClientset, namespace, selector) if err != nil { return err } @@ -68,7 +62,7 @@ func PodsWithSelector(kubeClientset kubernetes.Interface, namespace, selector st } func PodsWithSelectorHaveRestartCountLessThan(kubeClientset kubernetes.Interface, namespace string, selector string, restartCount int) error { - pods, err := ListPodsWithLabelSelector(kubeClientset, namespace, selector) + pods, err := GetPodListWithLabelSelector(kubeClientset, namespace, selector) if err != nil { return err } @@ -93,7 +87,7 @@ func PodsWithSelectorHaveRestartCountLessThan(kubeClientset kubernetes.Interface func SomeOrAllPodsInNamespaceWithSelectorHaveStringInLogsSinceTime(kubeClientset kubernetes.Interface, expBackoff wait.Backoff, SomeOrAll, namespace, selector, searchKeyword string, since time.Time) error { return util.RetryOnAnyError(&expBackoff, func() error { - pods, err := ListPodsWithLabelSelector(kubeClientset, namespace, selector) + pods, err := GetPodListWithLabelSelector(kubeClientset, namespace, selector) if err != nil { return err } @@ -134,7 +128,7 @@ func SomeOrAllPodsInNamespaceWithSelectorHaveStringInLogsSinceTime(kubeClientset } func SomePodsInNamespaceWithSelectorDontHaveStringInLogsSinceTime(kubeClientset kubernetes.Interface, namespace, selector, searchkeyword string, since time.Time) error { - pods, err := ListPodsWithLabelSelector(kubeClientset, namespace, selector) + pods, err := GetPodListWithLabelSelector(kubeClientset, namespace, selector) if err != nil { return err } @@ -155,7 +149,7 @@ func SomePodsInNamespaceWithSelectorDontHaveStringInLogsSinceTime(kubeClientset } func PodsInNamespaceWithSelectorHaveNoErrorsInLogsSinceTime(kubeClientset kubernetes.Interface, namespace string, selector string, since time.Time) error { - pods, err := ListPodsWithLabelSelector(kubeClientset, namespace, selector) + pods, err := GetPodListWithLabelSelector(kubeClientset, namespace, selector) if err != nil { return err } @@ -220,7 +214,7 @@ func PodInNamespaceShouldHaveLabels(kubeClientset kubernetes.Interface, name, na } func PodsInNamespaceWithSelectorShouldHaveLabels(kubeClientset kubernetes.Interface, namespace, selector, labels string) error { - podList, err := ListPodsWithLabelSelector(kubeClientset, namespace, selector) + podList, err := GetPodListWithLabelSelector(kubeClientset, namespace, selector) if err != nil { return fmt.Errorf("error getting pods with selector %q: %v", selector, err) } diff --git a/pkg/kube/pod/pod_helper.go b/pkg/kube/pod/pod_helper.go index fdcaeb5..6a0d1df 100644 --- a/pkg/kube/pod/pod_helper.go +++ b/pkg/kube/pod/pod_helper.go @@ -29,8 +29,7 @@ import ( "k8s.io/client-go/kubernetes" ) -// TODO: should be GetPodsWithLabelSelector because it returns the objects and not list/print them -func ListPodsWithLabelSelector(kubeClientset kubernetes.Interface, namespace, selector string) (*corev1.PodList, error) { +func GetPodListWithLabelSelector(kubeClientset kubernetes.Interface, namespace, selector string) (*corev1.PodList, error) { if err := common.ValidateClientset(kubeClientset); err != nil { return nil, err } diff --git a/pkg/kube/structured/structured.go b/pkg/kube/structured/structured.go index b436f55..af81065 100644 --- a/pkg/kube/structured/structured.go +++ b/pkg/kube/structured/structured.go @@ -138,7 +138,7 @@ func ClusterRbacIsFound(kubeClientset kubernetes.Interface, resourceType, name s return nil } -func GetNodes(kubeClientset kubernetes.Interface) error { +func ListNodes(kubeClientset kubernetes.Interface) error { var readyStatus = func(conditions []corev1.NodeCondition) string { var status = false @@ -158,7 +158,7 @@ func GetNodes(kubeClientset kubernetes.Interface) error { return "NotReady" } // List nodes - nodes, _ := ListNodes(kubeClientset) + nodes, _ := GetNodeList(kubeClientset) if nodes != nil { tableFormat := "%-64s%-12s%-24s%-16s" log.Infof(tableFormat, "NAME", "STATUS", "INSTANCEGROUP", "AZ") @@ -188,7 +188,7 @@ func DaemonSetIsRunning(kubeClientset kubernetes.Interface, expBackoff wait.Back }) if err != nil { // Print Pods after failure - _ = pod.Pods(kubeClientset, namespace) + _ = pod.ListPods(kubeClientset, namespace) return fmt.Errorf("daemonset '%s/%s' not updated: '%v'", namespace, name, err) } return nil @@ -227,7 +227,7 @@ func ValidatePrometheusVolumeClaimTemplatesName(kubeClientset kubernetes.Interfa // Validation required: // - To retain existing persistent volumes and not to loose any data. // - And avoid creating new name persistent volumes. - sfs, err := ListStatefulSets(kubeClientset, namespace) + sfs, err := GetStatefulSetList(kubeClientset, namespace) if err != nil { return err } diff --git a/pkg/kube/structured/structured_helper.go b/pkg/kube/structured/structured_helper.go index 4e33129..b4cfc60 100644 --- a/pkg/kube/structured/structured_helper.go +++ b/pkg/kube/structured/structured_helper.go @@ -30,7 +30,7 @@ import ( "k8s.io/client-go/kubernetes" ) -func ListNodes(kubeClientset kubernetes.Interface) (*corev1.NodeList, error) { +func GetNodeList(kubeClientset kubernetes.Interface) (*corev1.NodeList, error) { if err := common.ValidateClientset(kubeClientset); err != nil { return nil, err } @@ -87,8 +87,7 @@ func GetPersistentVolume(kubeClientset kubernetes.Interface, name string) (*core return pvs.(*corev1.PersistentVolume), nil } -// TODO: should be get not list because it returns the object instead printing it -func ListStatefulSets(kubeClientset kubernetes.Interface, namespace string) (*appsv1.StatefulSetList, error) { +func GetStatefulSetList(kubeClientset kubernetes.Interface, namespace string) (*appsv1.StatefulSetList, error) { if err := common.ValidateClientset(kubeClientset); err != nil { return nil, err } @@ -102,7 +101,7 @@ func ListStatefulSets(kubeClientset kubernetes.Interface, namespace string) (*ap return sts.(*appsv1.StatefulSetList), nil } -func ListPersistentVolumes(kubeClientset kubernetes.Interface) (*corev1.PersistentVolumeList, error) { +func GetPersistentVolumeList(kubeClientset kubernetes.Interface) (*corev1.PersistentVolumeList, error) { if err := common.ValidateClientset(kubeClientset); err != nil { return nil, err } @@ -167,7 +166,7 @@ func GetIngressEndpoint(kubeClientset kubernetes.Interface, w common.WaiterConfi // TODO: This is hardcoded based on prometheus names in IKS clusters. Might be worth making it more generic in the future func validatePrometheusPVLabels(kubeClientset kubernetes.Interface, volumeClaimTemplatesName string) error { // Get prometheus PersistentVolume list - pv, err := ListPersistentVolumes(kubeClientset) + pv, err := GetPersistentVolumeList(kubeClientset) if err != nil { log.Fatal(err) } diff --git a/pkg/kube/structured/structured_test.go b/pkg/kube/structured/structured_test.go index 04d3cb8..2e998f1 100644 --- a/pkg/kube/structured/structured_test.go +++ b/pkg/kube/structured/structured_test.go @@ -288,7 +288,7 @@ func TestClusterRbacIsFound(t *testing.T) { } } -func TestGetNodes(t *testing.T) { +func TestListNodes(t *testing.T) { type args struct { kubeClientset kubernetes.Interface } @@ -307,8 +307,8 @@ func TestGetNodes(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := GetNodes(tt.args.kubeClientset); (err != nil) != tt.wantErr { - t.Errorf("GetNodes() error = %v, wantErr %v", err, tt.wantErr) + if err := ListNodes(tt.args.kubeClientset); (err != nil) != tt.wantErr { + t.Errorf("ListNodes() error = %v, wantErr %v", err, tt.wantErr) } }) } diff --git a/pkg/kube/unstructured/unstructured.go b/pkg/kube/unstructured/unstructured.go index 55065d9..d374b6a 100644 --- a/pkg/kube/unstructured/unstructured.go +++ b/pkg/kube/unstructured/unstructured.go @@ -419,7 +419,7 @@ func DeleteResourcesAtPath(dynamicClient dynamic.Interface, dc discovery.Discove } func VerifyInstanceGroups(dynamicClient dynamic.Interface) error { - igs, err := ListInstanceGroups(dynamicClient) + igs, err := GetInstanceGroupList(dynamicClient) if err != nil { return err } diff --git a/pkg/kube/unstructured/unstructured_helper.go b/pkg/kube/unstructured/unstructured_helper.go index 5abfc9a..ea722af 100644 --- a/pkg/kube/unstructured/unstructured_helper.go +++ b/pkg/kube/unstructured/unstructured_helper.go @@ -72,7 +72,7 @@ func GetResources(dc discovery.DiscoveryInterface, TemplateArguments interface{} return resourceList, err } -func ListInstanceGroups(dynamicClient dynamic.Interface) (*unstructured.UnstructuredList, error) { +func GetInstanceGroupList(dynamicClient dynamic.Interface) (*unstructured.UnstructuredList, error) { const ( instanceGroupNamespace = "instance-manager" customResourceGroup = "instancemgr" diff --git a/pkg/kube/unstructured/unstructured_test.go b/pkg/kube/unstructured/unstructured_test.go index 54c0e7c..4cbb3c5 100644 --- a/pkg/kube/unstructured/unstructured_test.go +++ b/pkg/kube/unstructured/unstructured_test.go @@ -901,7 +901,7 @@ func TestGetResources(t *testing.T) { } } -func TestListInstanceGroups(t *testing.T) { +func TestGetInstanceGroupList(t *testing.T) { type args struct { dynamicClient dynamic.Interface } @@ -923,13 +923,13 @@ func TestListInstanceGroups(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ListInstanceGroups(tt.args.dynamicClient) + got, err := GetInstanceGroupList(tt.args.dynamicClient) if (err != nil) != tt.wantErr { - t.Errorf("ListInstanceGroups() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GetInstanceGroupList() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("ListInstanceGroups() = %s, want %s", util.StructToPrettyString(got), util.StructToPrettyString(tt.want)) + t.Errorf("GetInstanceGroupList() = %s, want %s", util.StructToPrettyString(got), util.StructToPrettyString(tt.want)) } }) }