Skip to content

Commit

Permalink
Proper use of verbs Get and List in functions/methods names
Browse files Browse the repository at this point in the history
  • Loading branch information
Alfredo Garo authored and Alfredo Garo committed Jul 22, 2023
1 parent be1f774 commit 4e77dab
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 45 deletions.
6 changes: 3 additions & 3 deletions kubedog.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
14 changes: 7 additions & 7 deletions pkg/kube/kube.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 9 additions & 15 deletions pkg/kube/pod/pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 Get<Resource>List 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)
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/kube/pod/pod_helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/kube/structured/structured.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
9 changes: 4 additions & 5 deletions pkg/kube/structured/structured_helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/kube/structured/structured_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/unstructured/unstructured.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/unstructured/unstructured_helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 4 additions & 4 deletions pkg/kube/unstructured/unstructured_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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))
}
})
}
Expand Down

0 comments on commit 4e77dab

Please sign in to comment.