From 3e8861aca2c3d26dffc8d2559324a4751f258d24 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 27 Feb 2018 14:48:33 +0000 Subject: [PATCH 01/12] Add seeds field --- docs/quick-start/cassandra-cluster.yaml | 1 + pkg/apis/navigator/types.go | 1 + pkg/apis/navigator/v1alpha1/types.go | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/docs/quick-start/cassandra-cluster.yaml b/docs/quick-start/cassandra-cluster.yaml index 003ba76e0..4da109b20 100644 --- a/docs/quick-start/cassandra-cluster.yaml +++ b/docs/quick-start/cassandra-cluster.yaml @@ -7,6 +7,7 @@ spec: nodePools: - name: "ringnodes" replicas: 3 + seeds: 2 datacenter: "demo-datacenter" rack: "demo-rack" persistence: diff --git a/pkg/apis/navigator/types.go b/pkg/apis/navigator/types.go index 9f63052e7..ad00c5445 100644 --- a/pkg/apis/navigator/types.go +++ b/pkg/apis/navigator/types.go @@ -43,6 +43,7 @@ type CassandraClusterNodePool struct { Datacenter string Resources v1.ResourceRequirements SchedulerName string + Seeds int } type CassandraClusterStatus struct { diff --git a/pkg/apis/navigator/v1alpha1/types.go b/pkg/apis/navigator/v1alpha1/types.go index d09825d02..ff5e402c2 100644 --- a/pkg/apis/navigator/v1alpha1/types.go +++ b/pkg/apis/navigator/v1alpha1/types.go @@ -74,6 +74,11 @@ type CassandraClusterNodePool struct { // If not specified, the pod will be dispatched by default scheduler. // +optional SchedulerName string `json:"schedulerName,omitempty"` + + // Seeds specifies the number of seed nodes to alocate in this nodepool. By + // default, 1 is selected. + // +optional + Seeds int `json:"seeds"` } type CassandraClusterStatus struct { From b51f489dffd2f00c786e8b60fe757f783f3f5f98 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 27 Feb 2018 14:49:18 +0000 Subject: [PATCH 02/12] Run make generate --- pkg/apis/navigator/v1alpha1/zz_generated.conversion.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go index a290cd478..815508afa 100644 --- a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go @@ -173,6 +173,7 @@ func autoConvert_v1alpha1_CassandraClusterNodePool_To_navigator_CassandraCluster out.Datacenter = in.Datacenter out.Resources = in.Resources out.SchedulerName = in.SchedulerName + out.Seeds = in.Seeds return nil } @@ -192,6 +193,7 @@ func autoConvert_navigator_CassandraClusterNodePool_To_v1alpha1_CassandraCluster out.Datacenter = in.Datacenter out.Resources = in.Resources out.SchedulerName = in.SchedulerName + out.Seeds = in.Seeds return nil } From 27f12637de2df68ae03c10e823aec1115fed9c36 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 27 Feb 2018 15:20:42 +0000 Subject: [PATCH 03/12] int -> int64 --- pkg/apis/navigator/types.go | 2 +- pkg/apis/navigator/v1alpha1/types.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/navigator/types.go b/pkg/apis/navigator/types.go index ad00c5445..461d3899e 100644 --- a/pkg/apis/navigator/types.go +++ b/pkg/apis/navigator/types.go @@ -43,7 +43,7 @@ type CassandraClusterNodePool struct { Datacenter string Resources v1.ResourceRequirements SchedulerName string - Seeds int + Seeds int64 } type CassandraClusterStatus struct { diff --git a/pkg/apis/navigator/v1alpha1/types.go b/pkg/apis/navigator/v1alpha1/types.go index ff5e402c2..1ce53c496 100644 --- a/pkg/apis/navigator/v1alpha1/types.go +++ b/pkg/apis/navigator/v1alpha1/types.go @@ -78,7 +78,7 @@ type CassandraClusterNodePool struct { // Seeds specifies the number of seed nodes to alocate in this nodepool. By // default, 1 is selected. // +optional - Seeds int `json:"seeds"` + Seeds int64 `json:"seeds"` } type CassandraClusterStatus struct { From 0748d0cc1cf355135d12b670537b6e87f9c7245f Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 27 Feb 2018 15:29:38 +0000 Subject: [PATCH 04/12] Add validation to number of seeds --- pkg/apis/navigator/validation/cassandra.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/apis/navigator/validation/cassandra.go b/pkg/apis/navigator/validation/cassandra.go index 559fc4cbe..11a37d004 100644 --- a/pkg/apis/navigator/validation/cassandra.go +++ b/pkg/apis/navigator/validation/cassandra.go @@ -1,6 +1,7 @@ package validation import ( + "fmt" "reflect" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" @@ -13,7 +14,18 @@ import ( func ValidateCassandraClusterNodePool(np *navigator.CassandraClusterNodePool, fldPath *field.Path) field.ErrorList { // TODO: call k8s.io/kubernetes/pkg/apis/core/validation.ValidateResourceRequirements on np.Resources // this will require vendoring kubernetes/kubernetes. - return field.ErrorList{} + + allErrs := field.ErrorList{} + if np.Seeds > np.Replicas { + allErrs = append(allErrs, + field.Invalid( + fldPath.Child("seeds"), + np.Seeds, + fmt.Sprintf("number of seeds cannot be greater than number of replicas (%d)", np.Replicas)), + ) + } + + return allErrs } func ValidateCassandraCluster(c *navigator.CassandraCluster) field.ErrorList { From 0689c071b4be608061a903f646abc331c41bfb6f Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 27 Feb 2018 16:36:43 +0000 Subject: [PATCH 05/12] Default number of seeds to 1 --- pkg/apis/navigator/validation/cassandra.go | 6 ++++++ pkg/registry/navigator/cassandracluster/strategy.go | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/pkg/apis/navigator/validation/cassandra.go b/pkg/apis/navigator/validation/cassandra.go index 11a37d004..dcaebeb37 100644 --- a/pkg/apis/navigator/validation/cassandra.go +++ b/pkg/apis/navigator/validation/cassandra.go @@ -25,6 +25,12 @@ func ValidateCassandraClusterNodePool(np *navigator.CassandraClusterNodePool, fl ) } + if np.Seeds < 0 { + allErrs = append(allErrs, + field.Invalid(fldPath.Child("seeds"), np.Seeds, "number of seeds must be 1 or greater"), + ) + } + return allErrs } diff --git a/pkg/registry/navigator/cassandracluster/strategy.go b/pkg/registry/navigator/cassandracluster/strategy.go index 55cc7e10e..a434233e8 100644 --- a/pkg/registry/navigator/cassandracluster/strategy.go +++ b/pkg/registry/navigator/cassandracluster/strategy.go @@ -66,10 +66,23 @@ func (cassandraClusterStrategy) NamespaceScoped() bool { return true } +func defaultSeeds(new *navigator.CassandraCluster) { + // default to 1 seed if not specified + for i := 0; i < len(new.Spec.NodePools); i++ { + if new.Spec.NodePools[i].Seeds == 0 { + new.Spec.NodePools[i].Seeds = 1 + } + } +} + func (cassandraClusterStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) { + new := obj.(*navigator.CassandraCluster) + defaultSeeds(new) } func (cassandraClusterStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) { + new := obj.(*navigator.CassandraCluster) + defaultSeeds(new) } func (cassandraClusterStrategy) Validate(ctx genericapirequest.Context, obj runtime.Object) field.ErrorList { From 8f6d4726960ba1cbb3f645c937a3b2d17f93763e Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Wed, 28 Feb 2018 10:54:07 +0000 Subject: [PATCH 06/12] Add e2e test for seeds=2 --- hack/e2e.sh | 41 +++++++++++++------ hack/testdata/cass-cluster-test.template.yaml | 1 + 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/hack/e2e.sh b/hack/e2e.sh index 831d041b5..e52760d27 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -222,6 +222,15 @@ if [[ "test_elasticsearchcluster" = "${TEST_PREFIX}"* ]]; then kube_delete_namespace_and_wait "${ES_TEST_NS}" fi +function apply_cassandracluster() { + kubectl apply \ + --namespace "${namespace}" \ + --filename \ + <(envsubst \ + '$NAVIGATOR_IMAGE_REPOSITORY:$NAVIGATOR_IMAGE_TAG:$NAVIGATOR_IMAGE_PULLPOLICY:$CASS_NAME:$CASS_REPLICAS:$CASS_VERSION:$CASS_SEEDS' \ + < "${SCRIPT_DIR}/testdata/cass-cluster-test.template.yaml") +} + function test_cassandracluster() { echo "Testing CassandraCluster" local namespace="${1}" @@ -230,6 +239,7 @@ function test_cassandracluster() { export CASS_REPLICAS=1 export CASS_CQL_PORT=9042 export CASS_VERSION="3.11.1" + export CASS_SEEDS=1 kube_create_namespace_with_quota "${namespace}" @@ -239,12 +249,7 @@ function test_cassandracluster() { fail_test "Failed to get cassandraclusters" fi - if ! kubectl apply \ - --namespace "${namespace}" \ - --filename \ - <(envsubst \ - '$NAVIGATOR_IMAGE_REPOSITORY:$NAVIGATOR_IMAGE_TAG:$NAVIGATOR_IMAGE_PULLPOLICY:$CASS_NAME:$CASS_REPLICAS:$CASS_VERSION' \ - < "${SCRIPT_DIR}/testdata/cass-cluster-test.template.yaml") + if ! apply_cassandracluster then fail_test "Failed to create cassandracluster" fi @@ -338,12 +343,10 @@ function test_cassandracluster() { # Increment the replica count export CASS_REPLICAS=2 - kubectl apply \ - --namespace "${namespace}" \ - --filename \ - <(envsubst \ - '$NAVIGATOR_IMAGE_REPOSITORY:$NAVIGATOR_IMAGE_TAG:$NAVIGATOR_IMAGE_PULLPOLICY:$CASS_NAME:$CASS_REPLICAS:$CASS_VERSION' \ - < "${SCRIPT_DIR}/testdata/cass-cluster-test.template.yaml") + if ! apply_cassandracluster + then + fail_test "Failed to apply cassandracluster" + fi if ! retry TIMEOUT=300 stdout_equals 2 kubectl \ --namespace "${namespace}" \ @@ -392,6 +395,20 @@ function test_cassandracluster() { then fail_test "Cassandra liveness probe failed to restart dead node" fi + + export CASS_REPLICAS=2 + export CASS_SEEDS=2 + if ! apply_cassandracluster + then + fail_test "Failed to apply cassandracluster" + fi + + seed_label=$(kubectl get pods --namespace "${namespace}" \ + cass-${CASS_NAME}-ringnodes-1 \ + -o jsonpath='{.metadata.labels.seed}') + if [ "$seed_label" != "true" ]; then + fail_test "Second cassandra node not marked as seed" + fi } if [[ "test_cassandracluster" = "${TEST_PREFIX}"* ]]; then diff --git a/hack/testdata/cass-cluster-test.template.yaml b/hack/testdata/cass-cluster-test.template.yaml index 5859a0406..5c0c77ef8 100644 --- a/hack/testdata/cass-cluster-test.template.yaml +++ b/hack/testdata/cass-cluster-test.template.yaml @@ -9,6 +9,7 @@ spec: replicas: ${CASS_REPLICAS} datacenter: "${CASS_NAME}-datacenter" rack: "{CASS_NAME}-rack" + seeds: ${CASS_SEEDS} persistence: enabled: true size: "5Gi" From 1b0dc0a2b5a6ff2bf535b3ff9a2b280703880139 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Wed, 28 Feb 2018 13:41:47 +0000 Subject: [PATCH 07/12] Move defaults to v1alpha1 --- pkg/apis/navigator/v1alpha1/defaults.go | 5 +++++ pkg/registry/navigator/cassandracluster/strategy.go | 13 ------------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/pkg/apis/navigator/v1alpha1/defaults.go b/pkg/apis/navigator/v1alpha1/defaults.go index 3bf61b535..9b9a2e7e4 100644 --- a/pkg/apis/navigator/v1alpha1/defaults.go +++ b/pkg/apis/navigator/v1alpha1/defaults.go @@ -20,4 +20,9 @@ func SetDefaults_CassandraClusterNodePool(np *CassandraClusterNodePool) { if np.Rack == "" { np.Rack = np.Name } + + // default to 1 seed if not specified + if np.Seeds == 0 { + np.Seeds = 1 + } } diff --git a/pkg/registry/navigator/cassandracluster/strategy.go b/pkg/registry/navigator/cassandracluster/strategy.go index a434233e8..55cc7e10e 100644 --- a/pkg/registry/navigator/cassandracluster/strategy.go +++ b/pkg/registry/navigator/cassandracluster/strategy.go @@ -66,23 +66,10 @@ func (cassandraClusterStrategy) NamespaceScoped() bool { return true } -func defaultSeeds(new *navigator.CassandraCluster) { - // default to 1 seed if not specified - for i := 0; i < len(new.Spec.NodePools); i++ { - if new.Spec.NodePools[i].Seeds == 0 { - new.Spec.NodePools[i].Seeds = 1 - } - } -} - func (cassandraClusterStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) { - new := obj.(*navigator.CassandraCluster) - defaultSeeds(new) } func (cassandraClusterStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) { - new := obj.(*navigator.CassandraCluster) - defaultSeeds(new) } func (cassandraClusterStrategy) Validate(ctx genericapirequest.Context, obj runtime.Object) field.ErrorList { From 4a9bdead38a60a52215859663f95a03395f88e41 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Thu, 1 Mar 2018 15:54:48 +0000 Subject: [PATCH 08/12] Rebase on top of new seed controller --- hack/e2e.sh | 9 +-- .../cassandra/seedlabeller/control.go | 64 ++++++++++++------- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/hack/e2e.sh b/hack/e2e.sh index e52760d27..c4e11c353 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -403,10 +403,11 @@ function test_cassandracluster() { fail_test "Failed to apply cassandracluster" fi - seed_label=$(kubectl get pods --namespace "${namespace}" \ - cass-${CASS_NAME}-ringnodes-1 \ - -o jsonpath='{.metadata.labels.seed}') - if [ "$seed_label" != "true" ]; then + if ! stdout_equals "cass-${CASS_NAME}-ringnodes-0 cass-${CASS_NAME}-ringnodes-1" \ + kubectl get pods --namespace "${namespace}" \ + --selector=navigator.jetstack.io/cassandra-seed=true \ + --output 'jsonpath={.items[*].metadata.name}' + then fail_test "Second cassandra node not marked as seed" fi } diff --git a/pkg/controllers/cassandra/seedlabeller/control.go b/pkg/controllers/cassandra/seedlabeller/control.go index 32ac9a5bf..ad6270902 100644 --- a/pkg/controllers/cassandra/seedlabeller/control.go +++ b/pkg/controllers/cassandra/seedlabeller/control.go @@ -44,36 +44,52 @@ func NewControl( func (c *defaultSeedLabeller) labelSeedNodes( cluster *v1alpha1.CassandraCluster, + np *v1alpha1.CassandraClusterNodePool, set *appsv1beta1.StatefulSet, ) error { - // TODO: make number of seed nodes configurable - pod, err := c.pods.Pods(cluster.Namespace).Get(fmt.Sprintf("%s-%d", set.Name, 0)) - if err != nil { - glog.Warningf("Couldn't get stateful set pod: %v", err) - return nil - } - labels := pod.Labels - value := labels[service.SeedLabelKey] - if value == service.SeedLabelValue { - return nil - } - if labels == nil { - labels = map[string]string{} + for i := int64(0); i < np.Replicas; i++ { + pod, err := c.pods.Pods(cluster.Namespace).Get(fmt.Sprintf("%s-%d", set.Name, i)) + if err != nil { + glog.Warningf("Couldn't get stateful set pod: %v", err) + return nil + } + + // default to not a seed + desiredLabel := "false" + + // label first n as seeds + if i < np.Seeds { + desiredLabel = seedprovider.SeedLabelValue + } + + labels := pod.Labels + value := labels[seedprovider.SeedLabelKey] + if value == desiredLabel { + continue + } + if labels == nil { + labels = map[string]string{} + } + labels[seedprovider.SeedLabelKey] = desiredLabel + podCopy := pod.DeepCopy() + podCopy.SetLabels(labels) + _, err = c.kubeClient.CoreV1().Pods(podCopy.Namespace).Update(podCopy) + if err != nil { + return err + } } - labels[service.SeedLabelKey] = service.SeedLabelValue - podCopy := pod.DeepCopy() - podCopy.SetLabels(labels) - _, err = c.kubeClient.CoreV1().Pods(podCopy.Namespace).Update(podCopy) - return err + return nil } func (c *defaultSeedLabeller) Sync(cluster *v1alpha1.CassandraCluster) error { - sets, err := util.StatefulSetsForCluster(cluster, c.statefulSetLister) - if err != nil { - return err - } - for _, s := range sets { - err = c.labelSeedNodes(cluster, s) + for _, np := range cluster.Spec.NodePools { + setName := util.NodePoolResourceName(cluster, &np) + + set, err := c.statefulSetLister.StatefulSets(cluster.Namespace).Get(setName) + if err != nil { + return err + } + err = c.labelSeedNodes(cluster, &np, set) if err != nil { return err } From 8503ea90ba941f12ef9e107ca1594b65a54fa6fe Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Mon, 5 Mar 2018 16:11:13 +0000 Subject: [PATCH 09/12] Add tests --- hack/e2e.sh | 4 +- pkg/apis/navigator/validation/cassandra.go | 2 +- .../cassandra/seedlabeller/control.go | 20 +++++++--- .../cassandra/seedlabeller/control_test.go | 40 ++++++++++++++++--- pkg/controllers/cassandra/testing/testing.go | 1 + 5 files changed, 53 insertions(+), 14 deletions(-) diff --git a/hack/e2e.sh b/hack/e2e.sh index c4e11c353..2bde2b6db 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -360,7 +360,7 @@ function test_cassandracluster() { # TODO: A better test would be to query the endpoints and check that only # the `-0` pods are included. E.g. # kubectl -n test-cassandra-1519754828-19864 get ep cass-cassandra-1519754828-19864-cassandra-seeds -o "jsonpath={.subsets[*].addresses[*].hostname}" - if ! stdout_equals "cass-${CASS_NAME}-ringnodes-0" \ + if ! retry stdout_equals "cass-${CASS_NAME}-ringnodes-0" \ kubectl get pods --namespace "${namespace}" \ --selector=navigator.jetstack.io/cassandra-seed=true \ --output 'jsonpath={.items[*].metadata.name}' @@ -403,7 +403,7 @@ function test_cassandracluster() { fail_test "Failed to apply cassandracluster" fi - if ! stdout_equals "cass-${CASS_NAME}-ringnodes-0 cass-${CASS_NAME}-ringnodes-1" \ + if ! retry stdout_equals "cass-${CASS_NAME}-ringnodes-0 cass-${CASS_NAME}-ringnodes-1" \ kubectl get pods --namespace "${namespace}" \ --selector=navigator.jetstack.io/cassandra-seed=true \ --output 'jsonpath={.items[*].metadata.name}' diff --git a/pkg/apis/navigator/validation/cassandra.go b/pkg/apis/navigator/validation/cassandra.go index dcaebeb37..83784a2da 100644 --- a/pkg/apis/navigator/validation/cassandra.go +++ b/pkg/apis/navigator/validation/cassandra.go @@ -27,7 +27,7 @@ func ValidateCassandraClusterNodePool(np *navigator.CassandraClusterNodePool, fl if np.Seeds < 0 { allErrs = append(allErrs, - field.Invalid(fldPath.Child("seeds"), np.Seeds, "number of seeds must be 1 or greater"), + field.Invalid(fldPath.Child("seeds"), np.Seeds, "number of seeds must be greater than or equal to 1"), ) } diff --git a/pkg/controllers/cassandra/seedlabeller/control.go b/pkg/controllers/cassandra/seedlabeller/control.go index ad6270902..1ad1bf7cf 100644 --- a/pkg/controllers/cassandra/seedlabeller/control.go +++ b/pkg/controllers/cassandra/seedlabeller/control.go @@ -54,11 +54,11 @@ func (c *defaultSeedLabeller) labelSeedNodes( return nil } - // default to not a seed - desiredLabel := "false" - // label first n as seeds - if i < np.Seeds { + isSeed := i < np.Seeds + + desiredLabel := "" + if isSeed { desiredLabel = seedprovider.SeedLabelValue } @@ -70,7 +70,13 @@ func (c *defaultSeedLabeller) labelSeedNodes( if labels == nil { labels = map[string]string{} } - labels[seedprovider.SeedLabelKey] = desiredLabel + + if isSeed { + labels[seedprovider.SeedLabelKey] = desiredLabel + } else { + delete(labels, seedprovider.SeedLabelKey) + } + podCopy := pod.DeepCopy() podCopy.SetLabels(labels) _, err = c.kubeClient.CoreV1().Pods(podCopy.Namespace).Update(podCopy) @@ -87,8 +93,10 @@ func (c *defaultSeedLabeller) Sync(cluster *v1alpha1.CassandraCluster) error { set, err := c.statefulSetLister.StatefulSets(cluster.Namespace).Get(setName) if err != nil { - return err + glog.Warningf("Couldn't get stateful set: %v", err) + return nil } + err = c.labelSeedNodes(cluster, &np, set) if err != nil { return err diff --git a/pkg/controllers/cassandra/seedlabeller/control_test.go b/pkg/controllers/cassandra/seedlabeller/control_test.go index fad694c16..f41ebc06c 100644 --- a/pkg/controllers/cassandra/seedlabeller/control_test.go +++ b/pkg/controllers/cassandra/seedlabeller/control_test.go @@ -16,7 +16,7 @@ import ( casstesting "github.com/jetstack/navigator/pkg/controllers/cassandra/testing" ) -func CheckSeedLabel(podName, podNamespace string, t *testing.T, state *controllers.State) { +func CheckSeedLabel(podName, seedLabelValue string, podNamespace string, t *testing.T, state *controllers.State) { p, err := state.Clientset. CoreV1(). Pods(podNamespace). @@ -24,7 +24,7 @@ func CheckSeedLabel(podName, podNamespace string, t *testing.T, state *controlle if err != nil { t.Fatal(err) } - if p.Labels[service.SeedLabelKey] != service.SeedLabelValue { + if p.Labels[service.SeedLabelKey] != seedLabelValue { t.Errorf("unexpected seed label: %s", p.Labels) } } @@ -44,6 +44,20 @@ func TestSeedLabellerSync(t *testing.T) { pod0ValueIncorrect := pod0LabelMissing.DeepCopy() pod0ValueIncorrect.Labels[service.SeedLabelKey] = "blah" + pod1 := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cass-cassandra-1-RingNodes-1", + Namespace: cluster.Namespace, + }, + } + + pod2 := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cass-cassandra-1-RingNodes-2", + Namespace: cluster.Namespace, + }, + } + type testT struct { kubeObjects []runtime.Object navObjects []runtime.Object @@ -63,7 +77,7 @@ func TestSeedLabellerSync(t *testing.T) { navObjects: []runtime.Object{cluster}, cluster: cluster, assertions: func(t *testing.T, state *controllers.State) { - CheckSeedLabel(pod0.Name, pod0.Namespace, t, state) + CheckSeedLabel(pod0.Name, seedprovider.SeedLabelValue, pod0.Namespace, t, state) }, }, "add label if key missing": { @@ -71,7 +85,7 @@ func TestSeedLabellerSync(t *testing.T) { navObjects: []runtime.Object{cluster}, cluster: cluster, assertions: func(t *testing.T, state *controllers.State) { - CheckSeedLabel(pod0.Name, pod0.Namespace, t, state) + CheckSeedLabel(pod0.Name, seedprovider.SeedLabelValue, pod0.Namespace, t, state) }, }, "fix label if value incorrect": { @@ -79,7 +93,23 @@ func TestSeedLabellerSync(t *testing.T) { navObjects: []runtime.Object{cluster}, cluster: cluster, assertions: func(t *testing.T, state *controllers.State) { - CheckSeedLabel(pod0.Name, pod0.Namespace, t, state) + CheckSeedLabel(pod0.Name, seedprovider.SeedLabelValue, pod0.Namespace, t, state) + }, + }, + "add multiple seeds": { + kubeObjects: []runtime.Object{ss0, pod0, pod1, pod2}, + navObjects: []runtime.Object{cluster}, + cluster: cluster, + assertions: func(t *testing.T, state *controllers.State) { + CheckSeedLabel(pod1.Name, seedprovider.SeedLabelValue, pod1.Namespace, t, state) + }, + }, + "don't add too many seeds": { + kubeObjects: []runtime.Object{ss0, pod0, pod1, pod2}, + navObjects: []runtime.Object{cluster}, + cluster: cluster, + assertions: func(t *testing.T, state *controllers.State) { + CheckSeedLabel(pod2.Name, "", pod2.Namespace, t, state) }, }, } diff --git a/pkg/controllers/cassandra/testing/testing.go b/pkg/controllers/cassandra/testing/testing.go index 91e2c4be3..23f62d9c7 100644 --- a/pkg/controllers/cassandra/testing/testing.go +++ b/pkg/controllers/cassandra/testing/testing.go @@ -37,6 +37,7 @@ func ClusterForTest() *v1alpha1.CassandraCluster { v1alpha1.CassandraClusterNodePool{ Name: "RingNodes", Replicas: 3, + Seeds: 2, }, }, }, From 507aae3f3ce367cc85a39c36158a7d71a6a633f2 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Thu, 8 Mar 2018 12:56:47 +0000 Subject: [PATCH 10/12] Make seeds field type *int64 --- pkg/apis/navigator/types.go | 2 +- pkg/apis/navigator/v1alpha1/defaults.go | 6 +++-- pkg/apis/navigator/v1alpha1/types.go | 2 +- .../v1alpha1/zz_generated.conversion.go | 4 +-- .../v1alpha1/zz_generated.deepcopy.go | 9 +++++++ pkg/apis/navigator/validation/cassandra.go | 27 ++++++++++--------- pkg/apis/navigator/zz_generated.deepcopy.go | 9 +++++++ .../cassandra/seedlabeller/control.go | 2 +- .../cassandra/seedlabeller/control_test.go | 12 +++++++++ pkg/controllers/cassandra/testing/testing.go | 3 ++- pkg/util/util.go | 4 +++ 11 files changed, 60 insertions(+), 20 deletions(-) diff --git a/pkg/apis/navigator/types.go b/pkg/apis/navigator/types.go index 461d3899e..869cb13d6 100644 --- a/pkg/apis/navigator/types.go +++ b/pkg/apis/navigator/types.go @@ -43,7 +43,7 @@ type CassandraClusterNodePool struct { Datacenter string Resources v1.ResourceRequirements SchedulerName string - Seeds int64 + Seeds *int64 } type CassandraClusterStatus struct { diff --git a/pkg/apis/navigator/v1alpha1/defaults.go b/pkg/apis/navigator/v1alpha1/defaults.go index 9b9a2e7e4..f98944f76 100644 --- a/pkg/apis/navigator/v1alpha1/defaults.go +++ b/pkg/apis/navigator/v1alpha1/defaults.go @@ -2,6 +2,8 @@ package v1alpha1 import ( "k8s.io/apimachinery/pkg/runtime" + + "github.com/jetstack/navigator/pkg/util" ) const ( @@ -22,7 +24,7 @@ func SetDefaults_CassandraClusterNodePool(np *CassandraClusterNodePool) { } // default to 1 seed if not specified - if np.Seeds == 0 { - np.Seeds = 1 + if np.Seeds == nil { + np.Seeds = util.Int64Ptr(1) } } diff --git a/pkg/apis/navigator/v1alpha1/types.go b/pkg/apis/navigator/v1alpha1/types.go index 1ce53c496..90c767371 100644 --- a/pkg/apis/navigator/v1alpha1/types.go +++ b/pkg/apis/navigator/v1alpha1/types.go @@ -78,7 +78,7 @@ type CassandraClusterNodePool struct { // Seeds specifies the number of seed nodes to alocate in this nodepool. By // default, 1 is selected. // +optional - Seeds int64 `json:"seeds"` + Seeds *int64 `json:"seeds,omitempty"` } type CassandraClusterStatus struct { diff --git a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go index 815508afa..8a02f1b71 100644 --- a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go @@ -173,7 +173,7 @@ func autoConvert_v1alpha1_CassandraClusterNodePool_To_navigator_CassandraCluster out.Datacenter = in.Datacenter out.Resources = in.Resources out.SchedulerName = in.SchedulerName - out.Seeds = in.Seeds + out.Seeds = (*int64)(unsafe.Pointer(in.Seeds)) return nil } @@ -193,7 +193,7 @@ func autoConvert_navigator_CassandraClusterNodePool_To_v1alpha1_CassandraCluster out.Datacenter = in.Datacenter out.Resources = in.Resources out.SchedulerName = in.SchedulerName - out.Seeds = in.Seeds + out.Seeds = (*int64)(unsafe.Pointer(in.Seeds)) return nil } diff --git a/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go index 0f15c581b..1b69c6921 100644 --- a/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go @@ -100,6 +100,15 @@ func (in *CassandraClusterNodePool) DeepCopyInto(out *CassandraClusterNodePool) } } in.Resources.DeepCopyInto(&out.Resources) + if in.Seeds != nil { + in, out := &in.Seeds, &out.Seeds + if *in == nil { + *out = nil + } else { + *out = new(int64) + **out = **in + } + } return } diff --git a/pkg/apis/navigator/validation/cassandra.go b/pkg/apis/navigator/validation/cassandra.go index 83784a2da..9ba9987fe 100644 --- a/pkg/apis/navigator/validation/cassandra.go +++ b/pkg/apis/navigator/validation/cassandra.go @@ -16,19 +16,22 @@ func ValidateCassandraClusterNodePool(np *navigator.CassandraClusterNodePool, fl // this will require vendoring kubernetes/kubernetes. allErrs := field.ErrorList{} - if np.Seeds > np.Replicas { - allErrs = append(allErrs, - field.Invalid( - fldPath.Child("seeds"), - np.Seeds, - fmt.Sprintf("number of seeds cannot be greater than number of replicas (%d)", np.Replicas)), - ) - } - if np.Seeds < 0 { - allErrs = append(allErrs, - field.Invalid(fldPath.Child("seeds"), np.Seeds, "number of seeds must be greater than or equal to 1"), - ) + if np.Seeds != nil { + if *np.Seeds > np.Replicas { + allErrs = append(allErrs, + field.Invalid( + fldPath.Child("seeds"), + np.Seeds, + fmt.Sprintf("number of seeds cannot be greater than number of replicas (%d)", np.Replicas)), + ) + } + + if *np.Seeds < 1 { + allErrs = append(allErrs, + field.Invalid(fldPath.Child("seeds"), np.Seeds, "number of seeds must be greater than or equal to 1"), + ) + } } return allErrs diff --git a/pkg/apis/navigator/zz_generated.deepcopy.go b/pkg/apis/navigator/zz_generated.deepcopy.go index e187c402e..3bbd0f0d7 100644 --- a/pkg/apis/navigator/zz_generated.deepcopy.go +++ b/pkg/apis/navigator/zz_generated.deepcopy.go @@ -100,6 +100,15 @@ func (in *CassandraClusterNodePool) DeepCopyInto(out *CassandraClusterNodePool) } } in.Resources.DeepCopyInto(&out.Resources) + if in.Seeds != nil { + in, out := &in.Seeds, &out.Seeds + if *in == nil { + *out = nil + } else { + *out = new(int64) + **out = **in + } + } return } diff --git a/pkg/controllers/cassandra/seedlabeller/control.go b/pkg/controllers/cassandra/seedlabeller/control.go index 1ad1bf7cf..9b889be48 100644 --- a/pkg/controllers/cassandra/seedlabeller/control.go +++ b/pkg/controllers/cassandra/seedlabeller/control.go @@ -55,7 +55,7 @@ func (c *defaultSeedLabeller) labelSeedNodes( } // label first n as seeds - isSeed := i < np.Seeds + isSeed := i < *np.Seeds desiredLabel := "" if isSeed { diff --git a/pkg/controllers/cassandra/seedlabeller/control_test.go b/pkg/controllers/cassandra/seedlabeller/control_test.go index f41ebc06c..c842b9f94 100644 --- a/pkg/controllers/cassandra/seedlabeller/control_test.go +++ b/pkg/controllers/cassandra/seedlabeller/control_test.go @@ -14,6 +14,7 @@ import ( "github.com/jetstack/navigator/pkg/controllers/cassandra/seedlabeller" "github.com/jetstack/navigator/pkg/controllers/cassandra/service" casstesting "github.com/jetstack/navigator/pkg/controllers/cassandra/testing" + "github.com/jetstack/navigator/pkg/util" ) func CheckSeedLabel(podName, seedLabelValue string, podNamespace string, t *testing.T, state *controllers.State) { @@ -44,6 +45,9 @@ func TestSeedLabellerSync(t *testing.T) { pod0ValueIncorrect := pod0LabelMissing.DeepCopy() pod0ValueIncorrect.Labels[service.SeedLabelKey] = "blah" + clusterOneSeed := cluster.DeepCopy() + clusterOneSeed.Spec.NodePools[0].Seeds = util.Int64Ptr(1) + pod1 := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "cass-cassandra-1-RingNodes-1", @@ -112,6 +116,14 @@ func TestSeedLabellerSync(t *testing.T) { CheckSeedLabel(pod2.Name, "", pod2.Namespace, t, state) }, }, + "delete label if seed number decreased": { + kubeObjects: []runtime.Object{ss0, pod0, pod1, pod2}, + navObjects: []runtime.Object{cluster}, + cluster: clusterOneSeed, + assertions: func(t *testing.T, state *controllers.State) { + CheckSeedLabel(pod1.Name, "", pod1.Namespace, t, state) + }, + }, } for title, test := range tests { t.Run( diff --git a/pkg/controllers/cassandra/testing/testing.go b/pkg/controllers/cassandra/testing/testing.go index 23f62d9c7..c592dad90 100644 --- a/pkg/controllers/cassandra/testing/testing.go +++ b/pkg/controllers/cassandra/testing/testing.go @@ -16,6 +16,7 @@ import ( "github.com/jetstack/navigator/pkg/controllers/cassandra/seedlabeller" "github.com/jetstack/navigator/pkg/controllers/cassandra/service" "github.com/jetstack/navigator/pkg/controllers/cassandra/serviceaccount" + "github.com/jetstack/navigator/pkg/util" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -37,7 +38,7 @@ func ClusterForTest() *v1alpha1.CassandraCluster { v1alpha1.CassandraClusterNodePool{ Name: "RingNodes", Replicas: 3, - Seeds: 2, + Seeds: util.Int64Ptr(2), }, }, }, diff --git a/pkg/util/util.go b/pkg/util/util.go index 88feaf709..f47904b9d 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -11,3 +11,7 @@ func CalculateQuorum(num int32) int32 { } return (num / 2) + 1 } + +func Int64Ptr(i int64) *int64 { + return &i +} From e13293fcd0096a8b07ac7796a1b70524d60042c7 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Fri, 9 Mar 2018 10:45:04 +0000 Subject: [PATCH 11/12] Fix typo --- pkg/apis/navigator/v1alpha1/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/navigator/v1alpha1/types.go b/pkg/apis/navigator/v1alpha1/types.go index 90c767371..fdff70901 100644 --- a/pkg/apis/navigator/v1alpha1/types.go +++ b/pkg/apis/navigator/v1alpha1/types.go @@ -75,7 +75,7 @@ type CassandraClusterNodePool struct { // +optional SchedulerName string `json:"schedulerName,omitempty"` - // Seeds specifies the number of seed nodes to alocate in this nodepool. By + // Seeds specifies the number of seed nodes to allocate in this nodepool. By // default, 1 is selected. // +optional Seeds *int64 `json:"seeds,omitempty"` From 3541a6d1264c5ca6e23419999700611923a9e3f2 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 27 Mar 2018 17:46:59 +0100 Subject: [PATCH 12/12] Rebase --- pkg/apis/navigator/types.go | 2 +- pkg/apis/navigator/v1alpha1/defaults.go | 4 ++-- pkg/apis/navigator/v1alpha1/types.go | 2 +- pkg/apis/navigator/v1alpha1/zz_generated.conversion.go | 4 ++-- pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go | 2 +- pkg/apis/navigator/zz_generated.deepcopy.go | 2 +- pkg/controllers/cassandra/seedlabeller/control.go | 10 +++++----- pkg/util/util.go | 4 ---- 8 files changed, 13 insertions(+), 17 deletions(-) diff --git a/pkg/apis/navigator/types.go b/pkg/apis/navigator/types.go index 869cb13d6..f3f09160a 100644 --- a/pkg/apis/navigator/types.go +++ b/pkg/apis/navigator/types.go @@ -43,7 +43,7 @@ type CassandraClusterNodePool struct { Datacenter string Resources v1.ResourceRequirements SchedulerName string - Seeds *int64 + Seeds *int32 } type CassandraClusterStatus struct { diff --git a/pkg/apis/navigator/v1alpha1/defaults.go b/pkg/apis/navigator/v1alpha1/defaults.go index f98944f76..2c51e694d 100644 --- a/pkg/apis/navigator/v1alpha1/defaults.go +++ b/pkg/apis/navigator/v1alpha1/defaults.go @@ -3,7 +3,7 @@ package v1alpha1 import ( "k8s.io/apimachinery/pkg/runtime" - "github.com/jetstack/navigator/pkg/util" + "github.com/jetstack/navigator/pkg/util/ptr" ) const ( @@ -25,6 +25,6 @@ func SetDefaults_CassandraClusterNodePool(np *CassandraClusterNodePool) { // default to 1 seed if not specified if np.Seeds == nil { - np.Seeds = util.Int64Ptr(1) + np.Seeds = ptr.Int32(1) } } diff --git a/pkg/apis/navigator/v1alpha1/types.go b/pkg/apis/navigator/v1alpha1/types.go index fdff70901..8cbd1e148 100644 --- a/pkg/apis/navigator/v1alpha1/types.go +++ b/pkg/apis/navigator/v1alpha1/types.go @@ -78,7 +78,7 @@ type CassandraClusterNodePool struct { // Seeds specifies the number of seed nodes to allocate in this nodepool. By // default, 1 is selected. // +optional - Seeds *int64 `json:"seeds,omitempty"` + Seeds *int32 `json:"seeds,omitempty"` } type CassandraClusterStatus struct { diff --git a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go index 8a02f1b71..79ecc34b9 100644 --- a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go @@ -173,7 +173,7 @@ func autoConvert_v1alpha1_CassandraClusterNodePool_To_navigator_CassandraCluster out.Datacenter = in.Datacenter out.Resources = in.Resources out.SchedulerName = in.SchedulerName - out.Seeds = (*int64)(unsafe.Pointer(in.Seeds)) + out.Seeds = (*int32)(unsafe.Pointer(in.Seeds)) return nil } @@ -193,7 +193,7 @@ func autoConvert_navigator_CassandraClusterNodePool_To_v1alpha1_CassandraCluster out.Datacenter = in.Datacenter out.Resources = in.Resources out.SchedulerName = in.SchedulerName - out.Seeds = (*int64)(unsafe.Pointer(in.Seeds)) + out.Seeds = (*int32)(unsafe.Pointer(in.Seeds)) return nil } diff --git a/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go index 1b69c6921..cc829c1bb 100644 --- a/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go @@ -105,7 +105,7 @@ func (in *CassandraClusterNodePool) DeepCopyInto(out *CassandraClusterNodePool) if *in == nil { *out = nil } else { - *out = new(int64) + *out = new(int32) **out = **in } } diff --git a/pkg/apis/navigator/zz_generated.deepcopy.go b/pkg/apis/navigator/zz_generated.deepcopy.go index 3bbd0f0d7..0b1e2f102 100644 --- a/pkg/apis/navigator/zz_generated.deepcopy.go +++ b/pkg/apis/navigator/zz_generated.deepcopy.go @@ -105,7 +105,7 @@ func (in *CassandraClusterNodePool) DeepCopyInto(out *CassandraClusterNodePool) if *in == nil { *out = nil } else { - *out = new(int64) + *out = new(int32) **out = **in } } diff --git a/pkg/controllers/cassandra/seedlabeller/control.go b/pkg/controllers/cassandra/seedlabeller/control.go index 9b889be48..091f11afa 100644 --- a/pkg/controllers/cassandra/seedlabeller/control.go +++ b/pkg/controllers/cassandra/seedlabeller/control.go @@ -47,7 +47,7 @@ func (c *defaultSeedLabeller) labelSeedNodes( np *v1alpha1.CassandraClusterNodePool, set *appsv1beta1.StatefulSet, ) error { - for i := int64(0); i < np.Replicas; i++ { + for i := int32(0); i < np.Replicas; i++ { pod, err := c.pods.Pods(cluster.Namespace).Get(fmt.Sprintf("%s-%d", set.Name, i)) if err != nil { glog.Warningf("Couldn't get stateful set pod: %v", err) @@ -59,11 +59,11 @@ func (c *defaultSeedLabeller) labelSeedNodes( desiredLabel := "" if isSeed { - desiredLabel = seedprovider.SeedLabelValue + desiredLabel = service.SeedLabelValue } labels := pod.Labels - value := labels[seedprovider.SeedLabelKey] + value := labels[service.SeedLabelKey] if value == desiredLabel { continue } @@ -72,9 +72,9 @@ func (c *defaultSeedLabeller) labelSeedNodes( } if isSeed { - labels[seedprovider.SeedLabelKey] = desiredLabel + labels[service.SeedLabelKey] = desiredLabel } else { - delete(labels, seedprovider.SeedLabelKey) + delete(labels, service.SeedLabelKey) } podCopy := pod.DeepCopy() diff --git a/pkg/util/util.go b/pkg/util/util.go index f47904b9d..88feaf709 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -11,7 +11,3 @@ func CalculateQuorum(num int32) int32 { } return (num / 2) + 1 } - -func Int64Ptr(i int64) *int64 { - return &i -}