From 739973191e96964bc7a394acc17cac46f9a694ff Mon Sep 17 00:00:00 2001 From: Jordi Massaguer Pla Date: Mon, 27 Jan 2020 16:36:06 +0100 Subject: [PATCH] Revert "fixes bsc#1157338 (#860)" This reverts commit 55026300ec1317a108003c643b69ae5db61972af. Because of https://github.com/SUSE/avant-garde/issues/1257 we need to revert https://github.com/SUSE/skuba/pull/860 (cherry picked from commit 59e19939b6766396e2749e59f2b0d5d9e3b3a782) --- internal/pkg/skuba/addons/addons.go | 10 - internal/pkg/skuba/addons/dex.go | 4 - internal/pkg/skuba/addons/gangway.go | 2 - internal/pkg/skuba/kubernetes/nodes.go | 5 - internal/pkg/skuba/kubernetes/nodes_test.go | 56 ---- internal/pkg/skuba/kubernetes/versions.go | 8 +- internal/pkg/skuba/replica/replica.go | 280 ---------------- internal/pkg/skuba/replica/replica_test.go | 338 -------------------- internal/pkg/skuba/replica/testing.go | 154 --------- pkg/skuba/actions/node/join/join.go | 9 - pkg/skuba/actions/node/remove/remove.go | 32 +- 11 files changed, 19 insertions(+), 879 deletions(-) delete mode 100644 internal/pkg/skuba/replica/replica.go delete mode 100644 internal/pkg/skuba/replica/replica_test.go delete mode 100644 internal/pkg/skuba/replica/testing.go diff --git a/internal/pkg/skuba/addons/addons.go b/internal/pkg/skuba/addons/addons.go index 15d648f753..4d0a3c4931 100644 --- a/internal/pkg/skuba/addons/addons.go +++ b/internal/pkg/skuba/addons/addons.go @@ -33,7 +33,6 @@ import ( "k8s.io/klog" "github.com/SUSE/skuba/internal/pkg/skuba/kubernetes" - "github.com/SUSE/skuba/internal/pkg/skuba/replica" "github.com/SUSE/skuba/internal/pkg/skuba/skuba" skubaconstants "github.com/SUSE/skuba/pkg/skuba" ) @@ -258,15 +257,6 @@ func (addon Addon) Apply(client clientset.Interface, addonConfiguration AddonCon klog.Errorf("failed to run kubectl apply: %s", combinedOutput) return err } - - replicaHelper, err := replica.NewHelper(client) - if err != nil { - return err - } - if err := replicaHelper.UpdateNodes(); err != nil { - return err - } - if addon.callbacks != nil { if err := addon.callbacks.afterApply(addonConfiguration, skubaConfiguration); err != nil { // TODO: should we rollback here? diff --git a/internal/pkg/skuba/addons/dex.go b/internal/pkg/skuba/addons/dex.go index 4a0f1a9bc6..6b7d46bc34 100644 --- a/internal/pkg/skuba/addons/dex.go +++ b/internal/pkg/skuba/addons/dex.go @@ -161,12 +161,8 @@ kind: Deployment metadata: name: oidc-dex namespace: kube-system - labels: - app: oidc-dex - caasp.suse.com/skuba-replica-ha: "true" spec: replicas: 3 - revisionHistoryLimit: 0 selector: matchLabels: app: oidc-dex diff --git a/internal/pkg/skuba/addons/gangway.go b/internal/pkg/skuba/addons/gangway.go index 0bd62e92df..7d67fe9a0f 100644 --- a/internal/pkg/skuba/addons/gangway.go +++ b/internal/pkg/skuba/addons/gangway.go @@ -124,10 +124,8 @@ metadata: namespace: kube-system labels: app: oidc-gangway - caasp.suse.com/skuba-replica-ha: "true" spec: replicas: 3 - revisionHistoryLimit: 0 selector: matchLabels: app: oidc-gangway diff --git a/internal/pkg/skuba/kubernetes/nodes.go b/internal/pkg/skuba/kubernetes/nodes.go index 6f9842e234..f6c92bdfc2 100644 --- a/internal/pkg/skuba/kubernetes/nodes.go +++ b/internal/pkg/skuba/kubernetes/nodes.go @@ -30,11 +30,6 @@ import ( kubectldrain "k8s.io/kubernetes/pkg/kubectl/drain" ) -// GetAllNodes returns the list of nodes -func GetAllNodes(client clientset.Interface) (*corev1.NodeList, error) { - return client.CoreV1().Nodes().List(metav1.ListOptions{}) -} - // GetControlPlaneNodes returns the list of master nodes by matching // "node-role.kubernetes.io/master" label. func GetControlPlaneNodes(client clientset.Interface) (*corev1.NodeList, error) { diff --git a/internal/pkg/skuba/kubernetes/nodes_test.go b/internal/pkg/skuba/kubernetes/nodes_test.go index 06bd402ed5..6e3ccc9160 100644 --- a/internal/pkg/skuba/kubernetes/nodes_test.go +++ b/internal/pkg/skuba/kubernetes/nodes_test.go @@ -82,62 +82,6 @@ func createNode(name string, isControlPlane bool, machineID string) corev1.Node } } -func TestGetAllNodes(t *testing.T) { - tests := []struct { - name string - fakeClientset *fake.Clientset - expect int - }{ - { - name: "get nodes when cluster has 1 master", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, - }, - }, - ), - expect: 1, - }, - { - name: "get control plane node when cluster has 1 master, 1 worker", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, - w1, - }, - }, - ), - expect: 2, - }, - { - name: "get all node when cluster has 2 master, 2 worker", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, - w1, w2, - }, - }, - ), - expect: 4, - }, - } - - for _, tt := range tests { - tt := tt // Parallel testing - t.Run(tt.name, func(t *testing.T) { - actual, _ := GetAllNodes(tt.fakeClientset) - actualSize := len(actual.Items) - if actualSize != tt.expect { - t.Errorf("returned node number (%d) does not match the expected one (%d)", actualSize, tt.expect) - return - } - }) - } -} - func TestGetControlPlaneNodes(t *testing.T) { tests := []struct { name string diff --git a/internal/pkg/skuba/kubernetes/versions.go b/internal/pkg/skuba/kubernetes/versions.go index f5788c45f7..ec7ba6afb5 100644 --- a/internal/pkg/skuba/kubernetes/versions.go +++ b/internal/pkg/skuba/kubernetes/versions.go @@ -93,8 +93,8 @@ var ( AddonsVersion: AddonsVersion{ Cilium: &AddonVersion{"1.5.3", 1}, Kured: &AddonVersion{"1.2.0-rev4", 1}, - Dex: &AddonVersion{"2.16.0", 4}, - Gangway: &AddonVersion{"3.1.0-rev4", 4}, + Dex: &AddonVersion{"2.16.0", 3}, + Gangway: &AddonVersion{"3.1.0-rev4", 3}, PSP: &AddonVersion{"", 1}, }, }, @@ -113,8 +113,8 @@ var ( AddonsVersion: AddonsVersion{ Cilium: &AddonVersion{"1.5.3", 1}, Kured: &AddonVersion{"1.2.0-rev4", 1}, - Dex: &AddonVersion{"2.16.0", 4}, - Gangway: &AddonVersion{"3.1.0-rev4", 4}, + Dex: &AddonVersion{"2.16.0", 3}, + Gangway: &AddonVersion{"3.1.0-rev4", 3}, PSP: &AddonVersion{"", 1}, }, }, diff --git a/internal/pkg/skuba/replica/replica.go b/internal/pkg/skuba/replica/replica.go deleted file mode 100644 index 664dfcd834..0000000000 --- a/internal/pkg/skuba/replica/replica.go +++ /dev/null @@ -1,280 +0,0 @@ -/* - * Copyright (c) 2019 SUSE LLC. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -package replica - -import ( - "fmt" - - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - clientset "k8s.io/client-go/kubernetes" - - "github.com/SUSE/skuba/internal/pkg/skuba/kubernetes" -) - -type affinity int - -const ( - preferred affinity = iota - required -) - -const ( - highAvailabilitylabel = "caasp.suse.com/skuba-replica-ha" - minSize = 2 - patchAffinityRemove = `[ - { - "op":"remove", - "path":"/spec/template/spec/affinity" - } - ]` - patchAffinityRequired = `{ - "spec": { - "template": { - "spec": { - "affinity": { - "podAntiAffinity": { - "requiredDuringSchedulingIgnoredDuringExecution": [ - { - "labelSelector": { - "matchExpressions": [ - { - "key": "app", - "operator": "In", - "values": [ - "%s" - ] - } - ] - }, - "topologyKey": "kubernetes.io/hostname" - } - ] - } - } - } - } - } - }` - patchAffinityPreferred = `{ - "spec": { - "template": { - "spec": { - "affinity": { - "podAntiAffinity": { - "preferredDuringSchedulingIgnoredDuringExecution": [ - { - "weight": 100, - "podAffinityTerm": { - "labelSelector": { - "matchExpressions": [ - { - "key": "app", - "operator": "In", - "values": [ - "%s" - ] - } - ] - }, - "topologyKey": "kubernetes.io/hostname" - } - } - ] - } - } - } - } - } - }` - patchReplicas = `{ - "spec": { - "replicas": %v - } - }` -) - -// Helper provide methods for replica update of deployment -// resource that has highAvailabilitylabel. -type Helper struct { - Client clientset.Interface - ClusterSize int - SelectDeployments appsv1.DeploymentList - Deployment appsv1.Deployment - ReplicaSize int - MinSize int -} - -// NewHelper creates a helper to update deployment replicas. -func NewHelper(client clientset.Interface) (*Helper, error) { - fromSelectors, err := client.AppsV1().Deployments(metav1.NamespaceSystem).List( - metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=true", highAvailabilitylabel), - }, - ) - if err != nil { - return nil, err - } - - node, err := kubernetes.GetAllNodes(client) - if err != nil { - return nil, err - } - - return &Helper{ - Client: client, - ClusterSize: len(node.Items), - MinSize: minSize, - SelectDeployments: *fromSelectors, - }, nil -} - -func (r *Helper) replaceAffinity(remove affinity, create affinity) (bool, error) { - // doRemove will remove only when affinity path exists - doRemove := false - affinity := r.Deployment.Spec.Template.Spec.Affinity - if affinity != nil && affinity.PodAntiAffinity != nil { - preferredList := len(r.Deployment.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution) - requiredList := len(r.Deployment.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution) - switch remove { - case preferred: - if preferredList != 0 { - doRemove = true - break - } - if requiredList != 0 { - return false, nil - } - case required: - if requiredList != 0 { - doRemove = true - break - } - if preferredList != 0 { - return false, nil - } - } - } - if doRemove { - if err := r.removeAffinity(); err != nil { - return false, err - } - } - - var affinityJSON string - switch create { - case required: - affinityJSON = fmt.Sprintf(patchAffinityRequired, r.Deployment.ObjectMeta.Name) - default: - affinityJSON = fmt.Sprintf(patchAffinityPreferred, r.Deployment.ObjectMeta.Name) - } - - _, err := r.Client.AppsV1().Deployments(metav1.NamespaceSystem).Patch(r.Deployment.ObjectMeta.Name, types.StrategicMergePatchType, []byte(affinityJSON)) - if err != nil { - return false, err - } - - return true, nil -} - -func (r *Helper) removeAffinity() error { - _, err := r.Client.AppsV1().Deployments(metav1.NamespaceSystem).Patch(r.Deployment.ObjectMeta.Name, types.JSONPatchType, []byte(patchAffinityRemove)) - if err != nil { - return err - } - return nil -} - -func (r *Helper) updateDeploymentReplica(size int) (*appsv1.Deployment, error) { - replicaJSON := fmt.Sprintf(patchReplicas, size) - return r.Client.AppsV1().Deployments(metav1.NamespaceSystem).Patch(r.Deployment.ObjectMeta.Name, types.StrategicMergePatchType, []byte(replicaJSON)) -} - -func (r *Helper) refreshReplicas() error { - // At least one node needs to be free of the affinity rules for pods to re-distribute. - // Sizing down replicas would make room for new pod resource. - // For example, during rolling upgrade if all node already has `required`, scheduler will `Pend` - // when tries to create a new deployment with `preferred`. This is due to the conflict to its original rule. - // But this ends up with less of replicas. So need the following patch to bring the replicas to the right size. - if _, err := r.updateDeploymentReplica(r.ClusterSize - 1); err != nil { - return err - } - if _, err := r.updateDeploymentReplica(r.ReplicaSize); err != nil { - return err - } - return nil -} - -// UpdateNodes patches for replicas affinity rules after adding of nodes. -// This updates affinity rule to preferredDuringSchedulingIgnoredDuringExecution of -// cluster size less then replica size, -// And updates affinity rule to requiredDuringSchedulingIgnoredDuringExecution when -// cluster size is equal or greater to replicas. -func (r *Helper) UpdateNodes() error { - for _, deployment := range r.SelectDeployments.Items { - r.Deployment = deployment - r.ReplicaSize = int(*r.Deployment.Spec.Replicas) - if r.ReplicaSize == 0 { - r.ReplicaSize = r.MinSize - } - - nodes := r.ClusterSize - isAffinityReplaced := false - var err error - switch { - case nodes >= r.ReplicaSize: - isAffinityReplaced, err = r.replaceAffinity(preferred, required) - if err != nil { - return err - } - case nodes >= r.MinSize: - isAffinityReplaced, err = r.replaceAffinity(required, preferred) - if err != nil { - return err - } - } - if !isAffinityReplaced && nodes <= r.ReplicaSize { - if err := r.refreshReplicas(); err != nil { - return err - } - } - } - return nil -} - -// UpdateDrainNodes patches for replicas affinity rules before removing of nodes. -// This updates affinity rule to preferredDuringSchedulingIgnoredDuringExecution when -// cluster size equal or less then replicas. -func (r *Helper) UpdateDrainNodes() error { - for _, deployment := range r.SelectDeployments.Items { - r.Deployment = deployment - r.ReplicaSize = int(*r.Deployment.Spec.Replicas) - if r.ClusterSize > r.ReplicaSize { - continue - } - - if _, err := r.replaceAffinity(required, preferred); err != nil { - return err - } - if err := r.refreshReplicas(); err != nil { - return err - } - } - return nil -} diff --git a/internal/pkg/skuba/replica/replica_test.go b/internal/pkg/skuba/replica/replica_test.go deleted file mode 100644 index 004fa11b21..0000000000 --- a/internal/pkg/skuba/replica/replica_test.go +++ /dev/null @@ -1,338 +0,0 @@ -/* - * Copyright (c) 2019 SUSE LLC. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -package replica - -import ( - "fmt" - "testing" - - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/fake" - - "github.com/SUSE/skuba/internal/pkg/skuba/kubernetes" -) - -func TestNewHelper(t *testing.T) { - t.Run("create new helper", func(t *testing.T) { - fakeClientset := fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1R2, - }, - }, - ) - - newReplica, err := NewHelper(fakeClientset) - if err != nil { - t.Errorf("error not expected, but an error was reported (%v)", err.Error()) - } - - nodes, err := kubernetes.GetAllNodes(fakeClientset) - if err != nil { - t.Errorf("error not expected, but an error was reported (%v)", err.Error()) - } - expect := len(nodes.Items) - if newReplica.ClusterSize != expect { - t.Errorf("returned cluster size (%v) does not match the expected one (%v)", newReplica.ClusterSize, expect) - } - - deployments, err := fakeClientset.AppsV1().Deployments(metav1.NamespaceSystem).List( - metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=true", highAvailabilitylabel), - }, - ) - if err != nil { - t.Errorf("error not expected, but an error was reported (%v)", err.Error()) - } - actual := len(newReplica.SelectDeployments.Items) - expect = len(deployments.Items) - if actual != expect { - t.Errorf("returned deployment size (%v) does not match the expected one (%v)", actual, expect) - } - }) -} - -func TestUpdateNodes(t *testing.T) { - tests := []struct { - name string - fakeClientset *fake.Clientset - }{ - { - name: "2nd master nodes joins cluster, deployments: [replica-0]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1R0, - }, - }, - ), - }, - { - name: "2nd master nodes joins cluster, deployments: [replica-2]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1R2, - }, - }, - ), - }, - { - name: "2nd master node joins cluster, deployments: [replica-2, replica-3]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1R2, d1R3, - }, - }, - ), - }, - { - name: "3rd master node joins cluster, deployments: [replica-2]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1RequiredR2, - }, - }, - ), - }, - { - name: "3rd master node joins cluster, deployments: [replica-2, replica-3]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1RequiredR2, d1PreferredR3, - }, - }, - ), - }, - { - name: "4th master node joins cluster", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, m4, - }, - }, - ), - }, - { - name: "5th master node joins cluster", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, m4, m5, - }, - }, - ), - }, - } - - for _, tt := range tests { - tt := tt // Parallel testing - t.Run(tt.name, func(t *testing.T) { - replicaHelper, _ := NewHelper(tt.fakeClientset) - err := replicaHelper.UpdateNodes() - if err != nil { - t.Errorf("error not expected when (%s), but an error was reported (%v)", tt.name, err.Error()) - } - - deployments, _ := tt.fakeClientset.AppsV1().Deployments(metav1.NamespaceSystem).List( - metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=true", highAvailabilitylabel), - }, - ) - for _, d := range deployments.Items { - replicaSize := int(*d.Spec.Replicas) - switch nodes := replicaHelper.ClusterSize; { - case nodes >= replicaSize: - if len(d.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution) == 0 { - t.Error("expect affinity \"requiredDuringSchedulingIgnoredDuringExecution\", but affinity not exist") - } - case nodes >= replicaHelper.MinSize: - if len(d.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution) == 0 { - t.Error("expect affinity \"preferredDuringSchedulingIgnoredDuringExecution\", but affinity not exist") - } - } - } - }) - } -} - -func TestUpdateDrainNodes(t *testing.T) { - tests := []struct { - name string - fakeClientset *fake.Clientset - }{ - { - name: "5 master in cluster", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, m4, m5, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1RequiredR3, - }, - }, - ), - }, - { - name: "4 master in cluster", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, m4, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1RequiredR3, - }, - }, - ), - }, - { - name: "3 master in cluster, deployments: [replica-3]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1RequiredR3, - }, - }, - ), - }, - { - name: "3 master in cluster, deployments: [replica-3, replica-3]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, m3, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1RequiredR3, d2RequiredR3, - }, - }, - ), - }, - { - name: "2 master in cluster, deployments: [replica-2]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1PreferredR2, - }, - }, - ), - }, - { - name: "2 master in cluster, 2 labeled deployment: [replica-2, replica-3]", - fakeClientset: fake.NewSimpleClientset( - &corev1.NodeList{ - Items: []corev1.Node{ - m1, m2, - }, - }, - &appsv1.DeploymentList{ - Items: []appsv1.Deployment{ - d1RequiredR2, d2PreferredR3, - }, - }, - ), - }, - } - - for _, tt := range tests { - tt := tt // Parallel testing - t.Run(tt.name, func(t *testing.T) { - replicaHelper, _ := NewHelper(tt.fakeClientset) - err := replicaHelper.UpdateDrainNodes() - if err != nil { - t.Errorf("error not expected when (%s), but an error was reported (%v)", tt.name, err.Error()) - } - - deployments, _ := tt.fakeClientset.AppsV1().Deployments(metav1.NamespaceSystem).List( - metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=true", highAvailabilitylabel), - }, - ) - for _, d := range deployments.Items { - replicaSize := int(*d.Spec.Replicas) - preferredList := len(d.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution) - requiredList := len(d.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution) - switch nodes := replicaHelper.ClusterSize; { - case nodes > replicaSize: - if requiredList == 0 { - t.Error("expect affinity \"requiredDuringSchedulingIgnoredDuringExecution\", but affinity not exist") - } - case nodes <= replicaSize: - if preferredList == 0 { - t.Error("expect affinity \"preferredDuringSchedulingIgnoredDuringExecution\" but affinity not exist") - } - } - } - }) - } -} diff --git a/internal/pkg/skuba/replica/testing.go b/internal/pkg/skuba/replica/testing.go deleted file mode 100644 index 8dbaf2f063..0000000000 --- a/internal/pkg/skuba/replica/testing.go +++ /dev/null @@ -1,154 +0,0 @@ -/* - * Copyright (c) 2019 SUSE LLC. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -package replica - -import ( - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -var m1 = createNode("master1") -var m2 = createNode("master2") -var m3 = createNode("master3") -var m4 = createNode("master4") -var m5 = createNode("master5") - -func createNode(name string) corev1.Node { - return corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: metav1.NamespaceSystem, - Labels: map[string]string{"node-role.kubernetes.io/master": ""}, - }, - } -} - -var d1R0 = createDeployment("deployment1", 0) -var d1R2 = createDeployment("deployment1", 2) -var d1R3 = createDeployment("deployment2", 3) - -func createDeployment(deployment string, replica int) appsv1.Deployment { - return appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: deployment, - Namespace: metav1.NamespaceSystem, - Labels: map[string]string{"caasp.suse.com/skuba-replica-ha": "true"}, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: func() *int32 { i := int32(replica); return &i }(), - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "t-1", - Image: "t-1", - }, - }, - }, - }, - }, - } -} - -var d1RequiredR2 = createDeploymentWithAffinityRequired("deployment1-withRequired", 2) -var d1RequiredR3 = createDeploymentWithAffinityRequired("deployment1-withRequired", 3) -var d2RequiredR3 = createDeploymentWithAffinityRequired("deployment2-withRequired", 3) - -func createDeploymentWithAffinityRequired(deployment string, replica int) appsv1.Deployment { - return appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: deployment, - Namespace: metav1.NamespaceSystem, - Labels: map[string]string{"caasp.suse.com/skuba-replica-ha": "true"}, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: func() *int32 { i := int32(replica); return &i }(), - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "t-1", - Image: "t-1", - }, - }, - Affinity: &corev1.Affinity{ - PodAntiAffinity: &corev1.PodAntiAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "test", - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - } -} - -var d1PreferredR2 = createDeploymentWithAffinityPreferred("deployment1-Preferred-2", 2) -var d1PreferredR3 = createDeploymentWithAffinityPreferred("deployment1-Preferred-3", 3) -var d2PreferredR3 = createDeploymentWithAffinityPreferred("deployment2-Preferred-3", 3) - -func createDeploymentWithAffinityPreferred(deployment string, replica int) appsv1.Deployment { - return appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: deployment, - Namespace: metav1.NamespaceSystem, - Labels: map[string]string{"caasp.suse.com/skuba-replica-ha": "true"}, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: func() *int32 { i := int32(replica); return &i }(), - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "t-1", - Image: "t-1", - }, - }, - Affinity: &corev1.Affinity{ - PodAntiAffinity: &corev1.PodAntiAffinity{ - PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{ - { - Weight: 100, - PodAffinityTerm: corev1.PodAffinityTerm{ - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "test", - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - } -} diff --git a/pkg/skuba/actions/node/join/join.go b/pkg/skuba/actions/node/join/join.go index 847c0355d9..7fbdf83711 100644 --- a/pkg/skuba/actions/node/join/join.go +++ b/pkg/skuba/actions/node/join/join.go @@ -39,7 +39,6 @@ import ( "github.com/SUSE/skuba/internal/pkg/skuba/kubeadm" "github.com/SUSE/skuba/internal/pkg/skuba/kubernetes" "github.com/SUSE/skuba/internal/pkg/skuba/node" - "github.com/SUSE/skuba/internal/pkg/skuba/replica" "github.com/SUSE/skuba/pkg/skuba" ) @@ -101,14 +100,6 @@ func Join(client clientset.Interface, joinConfiguration deployments.JoinConfigur } } - replicaHelper, err := replica.NewHelper(client) - if err != nil { - return err - } - if err := replicaHelper.UpdateNodes(); err != nil { - return err - } - fmt.Println("[join] node successfully joined the cluster") return nil } diff --git a/pkg/skuba/actions/node/remove/remove.go b/pkg/skuba/actions/node/remove/remove.go index 88fe561a74..529e91b584 100644 --- a/pkg/skuba/actions/node/remove/remove.go +++ b/pkg/skuba/actions/node/remove/remove.go @@ -24,12 +24,12 @@ import ( "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" + kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" "github.com/SUSE/skuba/internal/pkg/skuba/cni" "github.com/SUSE/skuba/internal/pkg/skuba/etcd" "github.com/SUSE/skuba/internal/pkg/skuba/kubeadm" "github.com/SUSE/skuba/internal/pkg/skuba/kubernetes" - "github.com/SUSE/skuba/internal/pkg/skuba/replica" ) // Remove removes a node from the cluster @@ -39,6 +39,20 @@ func Remove(client clientset.Interface, target string, drainTimeout time.Duratio return errors.Wrapf(err, "[remove-node] could not get node %s", target) } + if kubernetes.IsControlPlane(node) { + nodes, err := client.CoreV1().Nodes().List(metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=", kubeadmconstants.LabelNodeRoleMaster), + }) + + if err != nil { + return errors.Wrapf(err, "could not retrieve master node list") + } + + if len(nodes.Items) == 1 { + return errors.New("could not remove last master of the cluster") + } + } + currentClusterVersion, err := kubeadm.GetCurrentClusterVersion(client) if err != nil { return errors.Wrap(err, "could not retrieve the current cluster version") @@ -48,27 +62,11 @@ func Remove(client clientset.Interface, target string, drainTimeout time.Duratio var isControlPlane bool if isControlPlane = kubernetes.IsControlPlane(node); isControlPlane { - cp, err := kubernetes.GetControlPlaneNodes(client) - if err != nil { - return errors.Wrapf(err, "could not retrieve master node list") - } - if len(cp.Items) == 1 { - return errors.New("could not remove last master of the cluster") - } - fmt.Printf("[remove-node] removing control plane node %s (drain timeout: %s)\n", targetName, drainTimeout.String()) } else { fmt.Printf("[remove-node] removing worker node %s (drain timeout: %s)\n", targetName, drainTimeout.String()) } - replicaHelper, err := replica.NewHelper(client) - if err != nil { - return err - } - if err := replicaHelper.UpdateDrainNodes(); err != nil { - return errors.Wrap(err, "[remove-node] failed to update deployment replicas") - } - if err := kubernetes.DrainNode(client, node, drainTimeout); err != nil { return errors.Wrap(err, "[remove-node] could not drain node") }