Merge pull request #318 from rabbitmq/small-refactoring

Small refactor

Author: ChunyiLyu

api/v1beta1/rabbitmqcluster_types.go

@@ -18,7 +18,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	k8sresource "k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	runtime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime"
 )
 
 const (
@@ -325,13 +325,13 @@ func (cluster *RabbitmqCluster) AdditionalPluginEnabled(plugin Plugin) bool {
 	return false
 }
 
-func (rmqStatus *RabbitmqClusterStatus) SetConditions(resources []runtime.Object) {
+func (clusterStatus *RabbitmqClusterStatus) SetConditions(resources []runtime.Object) {
 	var oldAllPodsReadyCondition *status.RabbitmqClusterCondition
 	var oldClusterAvailableCondition *status.RabbitmqClusterCondition
 	var oldNoWarningsCondition *status.RabbitmqClusterCondition
 	var oldReconcileCondition *status.RabbitmqClusterCondition
 
-	for _, condition := range rmqStatus.Conditions {
+	for _, condition := range clusterStatus.Conditions {
 		switch condition.Type {
 		case status.AllReplicasReady:
 			oldAllPodsReadyCondition = condition.DeepCopy()
@@ -355,20 +355,20 @@ func (rmqStatus *RabbitmqClusterStatus) SetConditions(resources []runtime.Object
 		reconciledCondition = status.ReconcileSuccessCondition(corev1.ConditionUnknown, "Initialising", "")
 	}
 
-	rmqStatus.Conditions = []status.RabbitmqClusterCondition{
+	clusterStatus.Conditions = []status.RabbitmqClusterCondition{
 		allReplicasReadyCond,
 		clusterAvailableCond,
 		noWarningsCond,
 		reconciledCondition,
 	}
 }
 
-func (rmqStatus *RabbitmqClusterStatus) SetCondition(condType status.RabbitmqClusterConditionType,
+func (clusterStatus *RabbitmqClusterStatus) SetCondition(condType status.RabbitmqClusterConditionType,
 	condStatus corev1.ConditionStatus, reason string, messages ...string) {
-	for i := range rmqStatus.Conditions {
-		if rmqStatus.Conditions[i].Type == condType {
-			rmqStatus.Conditions[i].UpdateState(condStatus)
-			rmqStatus.Conditions[i].UpdateReason(reason, messages...)
+	for i := range clusterStatus.Conditions {
+		if clusterStatus.Conditions[i].Type == condType {
+			clusterStatus.Conditions[i].UpdateState(condStatus)
+			clusterStatus.Conditions[i].UpdateReason(reason, messages...)
 			break
 		}
 	}
@@ -383,8 +383,8 @@ type RabbitmqClusterList struct {
 	Items           []RabbitmqCluster `json:"items"`
 }
 
-func (r RabbitmqCluster) ChildResourceName(name string) string {
-	return strings.Join([]string{r.Name, "rabbitmq", name}, "-")
+func (cluster RabbitmqCluster) ChildResourceName(name string) string {
+	return strings.Join([]string{cluster.Name, "rabbitmq", name}, "-")
 }
 
 func init() {

api/v1beta1/rabbitmqcluster_types_test.go

@@ -16,7 +16,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	k8sresource "k8s.io/apimachinery/pkg/api/resource"
-	runtime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"golang.org/x/net/context"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -163,7 +163,7 @@ var _ = Describe("RabbitmqCluster", func() {
 						Image:           "rabbitmq-image-from-cr",
 						ImagePullSecret: "my-super-secret",
 						Service: RabbitmqClusterServiceSpec{
-							Type: corev1.ServiceType("this-is-a-service"),
+							Type: "this-is-a-service",
 							Annotations: map[string]string{
 								"myannotation": "is-set",
 							},
@@ -186,7 +186,7 @@ var _ = Describe("RabbitmqCluster", func() {
 							NodeAffinity: &corev1.NodeAffinity{
 								RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
 									NodeSelectorTerms: []corev1.NodeSelectorTerm{
-										corev1.NodeSelectorTerm{
+										{
 											MatchExpressions: []corev1.NodeSelectorRequirement{
 												{
 													Key:      "somekey",
@@ -201,7 +201,7 @@ var _ = Describe("RabbitmqCluster", func() {
 							},
 						},
 						Tolerations: []corev1.Toleration{
-							corev1.Toleration{
+							{
 								Key:      "mykey",
 								Operator: "NotEqual",
 								Value:    "myvalue",
@@ -325,15 +325,15 @@ var _ = Describe("RabbitmqCluster", func() {
 
 		It("updates an arbitrary condition", func() {
 			someCondition := status.RabbitmqClusterCondition{}
-			someCondition.Type = status.RabbitmqClusterConditionType("a-type")
+			someCondition.Type = "a-type"
 			someCondition.Reason = "whynot"
-			someCondition.Status = corev1.ConditionStatus("perhaps")
+			someCondition.Status = "perhaps"
 			someCondition.LastTransitionTime = metav1.Unix(10, 0)
 			rmqStatus := RabbitmqClusterStatus{
 				Conditions: []status.RabbitmqClusterCondition{someCondition},
 			}
 
-			rmqStatus.SetCondition(status.RabbitmqClusterConditionType("a-type"),
+			rmqStatus.SetCondition("a-type",
 				corev1.ConditionTrue, "some-reason", "my-message")
 
 			updatedCondition := rmqStatus.Conditions[0]

controllers/rabbitmqcluster_controller.go

@@ -459,13 +459,13 @@ func (r *RabbitmqClusterReconciler) prepareForDeletion(ctx context.Context, rabb
 			}
 			// Add label on all Pods to be picked up in pre-stop hook via Downward API
 			if err := r.addRabbitmqDeletionLabel(ctx, rabbitmqCluster); err != nil {
-				return fmt.Errorf("Failed to add deletion markers to RabbitmqCluster Pods: %s", err.Error())
+				return fmt.Errorf("failed to add deletion markers to RabbitmqCluster Pods: %s", err.Error())
 			}
 			// Delete StatefulSet immediately after changing pod labels to minimize risk of them respawning.
 			// There is a window where the StatefulSet could respawn Pods without the deletion label in this order.
 			// But we can't delete it before because the DownwardAPI doesn't update once a Pod enters Terminating.
 			if err := r.Client.Delete(ctx, sts); client.IgnoreNotFound(err) != nil {
-				return fmt.Errorf("Cannot delete StatefulSet: %s", err.Error())
+				return fmt.Errorf("cannot delete StatefulSet: %s", err.Error())
 			}
 
 			return nil
@@ -508,7 +508,7 @@ func (r *RabbitmqClusterReconciler) addRabbitmqDeletionLabel(ctx context.Context
 		pod := &pods.Items[i]
 		pod.Labels[resource.DeletionMarker] = "true"
 		if err := r.Client.Update(ctx, pod); client.IgnoreNotFound(err) != nil {
-			return fmt.Errorf("Cannot Update Pod %s in Namespace %s: %s", pod.Name, pod.Namespace, err.Error())
+			return fmt.Errorf("cannot Update Pod %s in Namespace %s: %s", pod.Name, pod.Namespace, err.Error())
 		}
 	}
 

internal/metadata/metadata_suite_test.go

@@ -20,12 +20,3 @@ func TestResource(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Metadata Suite")
 }
-
-func testLabels(labels map[string]string) {
-	ExpectWithOffset(1, labels).To(SatisfyAll(
-		HaveKeyWithValue("foo", "bar"),
-		HaveKeyWithValue("rabbitmq", "is-great"),
-		HaveKeyWithValue("foo/app.kubernetes.io", "edgecase"),
-		Not(HaveKey("app.kubernetes.io/foo")),
-	))
-}

internal/resource/client_service.go

@@ -52,7 +52,7 @@ func (builder *ClientServiceBuilder) Update(object runtime.Object) error {
 	service := object.(*corev1.Service)
 	builder.setAnnotations(service)
 	service.Labels = metadata.GetLabels(builder.Instance.Name, builder.Instance.Labels)
-	service.Spec.Type = corev1.ServiceType(builder.Instance.Spec.Service.Type)
+	service.Spec.Type = builder.Instance.Spec.Service.Type
 	service.Spec.Selector = metadata.LabelSelector(builder.Instance.Name)
 
 	service.Spec.Ports = builder.updatePorts(service.Spec.Ports)

internal/resource/client_service_test.go

@@ -334,13 +334,13 @@ var _ = Context("ClientServices", func() {
 			It("preserves the same node ports after updating from LoadBalancer to NodePort", func() {
 				svc.Spec.Type = corev1.ServiceTypeLoadBalancer
 				svc.Spec.Ports = []corev1.ServicePort{
-					corev1.ServicePort{
+					{
 						Protocol: corev1.ProtocolTCP,
 						Port:     5672,
 						Name:     "amqp",
 						NodePort: 12345,
 					},
-					corev1.ServicePort{
+					{
 						Protocol: corev1.ProtocolTCP,
 						Port:     15672,
 						Name:     "management",
@@ -372,7 +372,7 @@ var _ = Context("ClientServices", func() {
 			It("unsets nodePort after updating from NodePort to ClusterIP", func() {
 				svc.Spec.Type = corev1.ServiceTypeNodePort
 				svc.Spec.Ports = []corev1.ServicePort{
-					corev1.ServicePort{
+					{
 						Protocol: corev1.ProtocolTCP,
 						Port:     5672,
 						Name:     "amqp",
@@ -399,7 +399,7 @@ var _ = Context("ClientServices", func() {
 			It("unsets the service type and node ports when service type is deleted from CR spec", func() {
 				svc.Spec.Type = corev1.ServiceTypeNodePort
 				svc.Spec.Ports = []corev1.ServicePort{
-					corev1.ServicePort{
+					{
 						Protocol: corev1.ProtocolTCP,
 						Port:     5672,
 						Name:     "amqp",

internal/resource/statefulset_test.go

@@ -1409,7 +1409,7 @@ func generateRabbitmqCluster() rabbitmqv1beta1.RabbitmqCluster {
 			Image:           "rabbitmq-image-from-cr",
 			ImagePullSecret: "my-super-secret",
 			Service: rabbitmqv1beta1.RabbitmqClusterServiceSpec{
-				Type:        corev1.ServiceType("this-is-a-service"),
+				Type:        "this-is-a-service",
 				Annotations: map[string]string{},
 			},
 			Persistence: rabbitmqv1beta1.RabbitmqClusterPersistenceSpec{
@@ -1430,7 +1430,7 @@ func generateRabbitmqCluster() rabbitmqv1beta1.RabbitmqCluster {
 				NodeAffinity: &corev1.NodeAffinity{
 					RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
 						NodeSelectorTerms: []corev1.NodeSelectorTerm{
-							corev1.NodeSelectorTerm{
+							{
 								MatchExpressions: []corev1.NodeSelectorRequirement{
 									{
 										Key:      "somekey",
@@ -1445,7 +1445,7 @@ func generateRabbitmqCluster() rabbitmqv1beta1.RabbitmqCluster {
 				},
 			},
 			Tolerations: []corev1.Toleration{
-				corev1.Toleration{
+				{
 					Key:      "mykey",
 					Operator: "NotEqual",
 					Value:    "myvalue",

internal/status/status_test.go

@@ -20,16 +20,16 @@ var _ = Describe("Status", func() {
 		BeforeEach(func() {
 			someConditionTime = metav1.Unix(1, 1)
 			someCondition = RabbitmqClusterCondition{
-				Type:               RabbitmqClusterConditionType("a-type"),
-				Status:             corev1.ConditionStatus("some-status"),
+				Type:               "a-type",
+				Status:             "some-status",
 				LastTransitionTime: (*someConditionTime.DeepCopy()),
 				Reason:             "reasons",
 				Message:            "ship-it",
 			}
 		})
 
 		It("changes the status and transition time", func() {
-			someCondition.UpdateState(corev1.ConditionStatus("maybe"))
+			someCondition.UpdateState("maybe")
 			Expect(someCondition.Status).To(Equal(corev1.ConditionStatus("maybe")))
 
 			Expect(someCondition.LastTransitionTime).NotTo(Equal(someConditionTime))
@@ -38,7 +38,7 @@ var _ = Describe("Status", func() {
 		})
 
 		It("preserves the status and transtion time", func() {
-			someCondition.UpdateState(corev1.ConditionStatus("some-status"))
+			someCondition.UpdateState("some-status")
 			Expect(someCondition.Status).To(Equal(corev1.ConditionStatus("some-status")))
 			Expect(someCondition.LastTransitionTime).To(Equal(someConditionTime))
 		})

system_tests/system_tests.go

@@ -23,10 +23,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const (
-	clientServiceSuffix = "client"
-	statefulSetSuffix   = "server"
-)
+const statefulSetSuffix   = "server"
 
 var _ = Describe("Operator", func() {
 	var (

system_tests/utils.go

@@ -600,7 +600,7 @@ func createTLSSecret(secretName, secretNamespace, hostname string) string {
 	// generate and write cert and key to file
 	Expect(createCertificateChain(hostname, caCertFile, serverCertFile, serverKeyFile)).To(Succeed())
 	// create k8s tls secret
-	Expect(k8sCreateSecretTLS("rabbitmq-tls-test-secret", secretNamespace, serverCertPath, serverKeyPath)).To(Succeed())
+	Expect(k8sCreateSecretTLS(secretName, secretNamespace, serverCertPath, serverKeyPath)).To(Succeed())
 
 	// remove server files
 	Expect(os.Remove(serverKeyPath)).To(Succeed())