Improve keepalived handling during updates

api/v1beta1/loadbalancerset_types.go

@@ -11,6 +11,7 @@ import (
 // +kubebuilder:printcolumn:name="DESIRED",type=string,JSONPath=`.spec.replicas`
 // +kubebuilder:printcolumn:name="CURRENT",type=string,JSONPath=`.status.replicas`
 // +kubebuilder:printcolumn:name="READY",type=string,JSONPath=`.status.readyReplicas`
+// +kubebuilder:printcolumn:name="HasKeepalivedMaster",type=string,JSONPath=`.status.conditions[?(@.type=="HasKeepalivedMaster")].status`
 // +kubebuilder:printcolumn:name="AGE",type=date,JSONPath=`.metadata.creationTimestamp`
 
 // LoadBalancerSet is the Schema for the LoadBalancerSet's API.
@@ -48,6 +49,9 @@ type LoadBalancerSetStatus struct {
 	// AvailableReplicas are the current running replicas.
 	// +optional
 	AvailableReplicas *int `json:"availableReplicas,omitempty"`
+	// Conditions contains condition information for a LoadBalancerSet.
+	// +optional
+	Conditions []metav1.Condition `json:"conditions,omitempty"`
 	// ReadyReplicas are the current ready replicas.
 	// +optional
 	ReadyReplicas *int `json:"readyReplicas,omitempty"`

charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancersets.yaml

@@ -27,6 +27,9 @@ spec:
     - jsonPath: .status.readyReplicas
       name: READY
       type: string
+    - jsonPath: .status.conditions[?(@.type=="HasKeepalivedMaster")].status
+      name: HasKeepalivedMaster
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: AGE
       type: date
@@ -289,6 +292,75 @@ spec:
               availableReplicas:
                 description: AvailableReplicas are the current running replicas.
                 type: integer
+              conditions:
+                description: Conditions contains condition information for a LoadBalancerSet.
+                items:
+                  description: "Condition contains details for one aspect of the current
+                    state of this API Resource. --- This struct is intended for direct
+                    use as an array at the field path .status.conditions.  For example,
+                    \n type FooStatus struct{ // Represents the observations of a
+                    foo's current state. // Known .status.conditions.type are: \"Available\",
+                    \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge
+                    // +listType=map // +listMapKey=type Conditions []metav1.Condition
+                    `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\"
+                    protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }"
+                  properties:
+                    lastTransitionTime:
+                      description: lastTransitionTime is the last time the condition
+                        transitioned from one status to another. This should be when
+                        the underlying condition changed.  If that is not known, then
+                        using the time when the API field changed is acceptable.
+                      format: date-time
+                      type: string
+                    message:
+                      description: message is a human readable message indicating
+                        details about the transition. This may be an empty string.
+                      maxLength: 32768
+                      type: string
+                    observedGeneration:
+                      description: observedGeneration represents the .metadata.generation
+                        that the condition was set based upon. For instance, if .metadata.generation
+                        is currently 12, but the .status.conditions[x].observedGeneration
+                        is 9, the condition is out of date with respect to the current
+                        state of the instance.
+                      format: int64
+                      minimum: 0
+                      type: integer
+                    reason:
+                      description: reason contains a programmatic identifier indicating
+                        the reason for the condition's last transition. Producers
+                        of specific condition types may define expected values and
+                        meanings for this field, and whether the values are considered
+                        a guaranteed API. The value should be a CamelCase string.
+                        This field may not be empty.
+                      maxLength: 1024
+                      minLength: 1
+                      pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
+                      type: string
+                    status:
+                      description: status of the condition, one of True, False, Unknown.
+                      enum:
+                      - "True"
+                      - "False"
+                      - Unknown
+                      type: string
+                    type:
+                      description: type of condition in CamelCase or in foo.example.com/CamelCase.
+                        --- Many .condition.type values are consistent across resources
+                        like Available, but because arbitrary conditions can be useful
+                        (see .node.status.conditions), the ability to deconflict is
+                        important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
+                      maxLength: 316
+                      pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
+                      type: string
+                  required:
+                  - lastTransitionTime
+                  - message
+                  - reason
+                  - status
+                  - type
+                  type: object
+                type: array
               readyReplicas:
                 description: ReadyReplicas are the current ready replicas.
                 type: integer

controllers/yawol-controller/loadbalancer/loadbalancer_controller.go

@@ -32,7 +32,6 @@ import (
 
 const (
 	DefaultRequeueTime = 10 * time.Millisecond
-	RevisionAnnotation = "loadbalancer.yawol.stackit.cloud/revision"
 	ServiceFinalizer   = "yawol.stackit.cloud/controller2"
 )
 
@@ -983,6 +982,9 @@ func (r *Reconciler) reconcileLoadBalancerSet(
 	}
 
 	if !downscaled {
+		if !helper.LBSetHasKeepalivedMaster(loadBalancerSet) {
+			return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
+		}
 		r.Log.Info("scale down all lbsets except of", "lbs", loadBalancerSet.Name)
 		return helper.ScaleDownAllLoadBalancerSetsForLBBut(ctx, r.Client, lb, loadBalancerSet.Name)
 	}

controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go

@@ -3,6 +3,7 @@ package loadbalancer
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -17,14 +18,14 @@ import (
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
 
 	yawolv1beta1 "github.com/stackitcloud/yawol/api/v1beta1"
+	"github.com/stackitcloud/yawol/internal/helper"
 	"github.com/stackitcloud/yawol/internal/openstack"
 	"github.com/stackitcloud/yawol/internal/openstack/testing"
 	v1 "k8s.io/api/core/v1"
 
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/pointer"
 	runtimeClient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -176,10 +177,7 @@ var _ = Describe("loadbalancer controller", Serial, Ordered, func() {
 		It("should create a loadbalancerset", func() {
 			hopefully(lbNN, func(g Gomega, act LB) error {
 				var lbsetList yawolv1beta1.LoadBalancerSetList
-				g.Expect(k8sClient.List(ctx, &lbsetList, &runtimeClient.ListOptions{
-					LabelSelector: labels.SelectorFromSet(lb.Spec.Selector.MatchLabels),
-				})).Should(Succeed())
-
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
 				g.Expect(len(lbsetList.Items)).Should(Equal(1))
 
 				// prevent later panic
@@ -204,10 +202,7 @@ var _ = Describe("loadbalancer controller", Serial, Ordered, func() {
 			By("waiting for lbset creation")
 			hopefully(lbNN, func(g Gomega, act LB) error {
 				var lbsetList yawolv1beta1.LoadBalancerSetList
-				g.Expect(k8sClient.List(ctx, &lbsetList, &runtimeClient.ListOptions{
-					LabelSelector: labels.SelectorFromSet(lb.Spec.Selector.MatchLabels),
-				})).Should(Succeed())
-
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
 				g.Expect(len(lbsetList.Items)).Should(Equal(1))
 
 				lbset := lbsetList.Items[0]
@@ -227,10 +222,7 @@ var _ = Describe("loadbalancer controller", Serial, Ordered, func() {
 			By("checking for a new lbset")
 			hopefully(lbNN, func(g Gomega, act LB) error {
 				var lbsetList yawolv1beta1.LoadBalancerSetList
-				g.Expect(k8sClient.List(ctx, &lbsetList, &runtimeClient.ListOptions{
-					LabelSelector: labels.SelectorFromSet(lb.Spec.Selector.MatchLabels),
-				})).Should(Succeed())
-
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
 				g.Expect(len(lbsetList.Items)).Should(Equal(2))
 
 				By("testing if the new set got a different hash")
@@ -248,14 +240,117 @@ var _ = Describe("loadbalancer controller", Serial, Ordered, func() {
 			})
 		})
 
+		It("should scale down old lbset after new one has ready keepalived", func() {
+			var oldLbs runtimeClient.ObjectKey
+			By("waiting for lbset creation")
+			hopefully(lbNN, func(g Gomega, act LB) error {
+				var lbsetList yawolv1beta1.LoadBalancerSetList
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
+				g.Expect(len(lbsetList.Items)).Should(Equal(1))
+				oldLbs = runtimeClient.ObjectKeyFromObject(&lbsetList.Items[0])
+				return nil
+			})
+
+			By("changing flavorid")
+			updateLB(lbNN, func(act *LB) {
+				act.Spec.Infrastructure.Flavor = yawolv1beta1.OpenstackFlavorRef{
+					FlavorID: pointer.String("somenewid"),
+				}
+			})
+
+			var newLbs runtimeClient.ObjectKey
+			By("checking for a new lbset")
+			hopefully(lbNN, func(g Gomega, act LB) error {
+				var lbsetList yawolv1beta1.LoadBalancerSetList
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
+				g.Expect(len(lbsetList.Items)).Should(Equal(2))
+				for _, lbs := range lbsetList.Items {
+					if lbs.Annotations[helper.RevisionAnnotation] == "2" {
+						newLbs = runtimeClient.ObjectKeyFromObject(&lbs)
+					}
+				}
+				return nil
+			})
+
+			By("Make both lbsets available by patching status")
+			var lbsetList yawolv1beta1.LoadBalancerSetList
+			Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
+			for _, lbs := range lbsetList.Items {
+				patch := runtimeClient.MergeFrom(lbs.DeepCopy())
+				lbs.Status.ReadyReplicas = &lbs.Spec.Replicas
+				lbs.Status.Replicas = &lbs.Spec.Replicas
+				Expect(k8sClient.Status().Patch(ctx, &lbs, patch)).Should(Succeed())
+			}
+
+			By("Old lbset should be scaled up because keepalived is not ready now")
+			Consistently(func() error {
+				var lbs yawolv1beta1.LoadBalancerSet
+				if err := k8sClient.Get(ctx, oldLbs, &lbs); err != nil {
+					return err
+				}
+				if lbs.Spec.Replicas == 0 {
+					return errors.New("already down scaled")
+				}
+
+				return nil
+			}, time.Second*8).Should(Succeed())
+
+			By("Make latest lbsets keepalived condition true by patching status")
+			var lbs yawolv1beta1.LoadBalancerSet
+			Expect(k8sClient.Get(ctx, newLbs, &lbs)).Should(Succeed())
+			patch := runtimeClient.MergeFrom(lbs.DeepCopy())
+			lbs.Status.Conditions = []metav1.Condition{
+				{
+					Type:               helper.HasKeepalivedMaster,
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: metav1.Now(),
+					Reason:             "ready",
+					Message:            "ready",
+				},
+			}
+			Expect(k8sClient.Status().Patch(ctx, &lbs, patch)).Should(Succeed())
+
+			By("Old lbset should be scaled up because keepalived is not ready long enough")
+			Consistently(func() error {
+				var lbs yawolv1beta1.LoadBalancerSet
+				if err := k8sClient.Get(ctx, oldLbs, &lbs); err != nil {
+					return err
+				}
+				if lbs.Spec.Replicas == 0 {
+					return errors.New("already down scaled")
+				}
+
+				return nil
+			}, time.Second*8).Should(Succeed())
+
+			By("Make latest lbsets keepalived condition true since longer")
+			Expect(k8sClient.Get(ctx, newLbs, &lbs)).Should(Succeed())
+			patch = runtimeClient.MergeFrom(lbs.DeepCopy())
+			lbs.Status.Conditions = []metav1.Condition{
+				{
+					Type:               helper.HasKeepalivedMaster,
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: metav1.Time{Time: metav1.Now().Add(-120 * time.Second)},
+					Reason:             "ready",
+					Message:            "ready",
+				},
+			}
+			Expect(k8sClient.Status().Patch(ctx, &lbs, patch)).Should(Succeed())
+
+			By("Old lbs should be downscaled")
+			hopefully(lbNN, func(g Gomega, act LB) error {
+				var lbs yawolv1beta1.LoadBalancerSet
+				g.Expect(k8sClient.Get(ctx, oldLbs, &lbs)).Should(Succeed())
+				g.Expect(lbs.Spec.Replicas).To(Equal(0))
+				return nil
+			})
+		})
+
 		It("should up- and downscale loadbalancer machines", func() {
 			By("waiting for lb and lbset creation")
 			hopefully(lbNN, func(g Gomega, act LB) error {
 				var lbsetList yawolv1beta1.LoadBalancerSetList
-				g.Expect(k8sClient.List(ctx, &lbsetList, &runtimeClient.ListOptions{
-					LabelSelector: labels.SelectorFromSet(lb.Spec.Selector.MatchLabels),
-				})).Should(Succeed())
-
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
 				g.Expect(len(lbsetList.Items)).Should(Equal(1))
 
 				lbset := lbsetList.Items[0]
@@ -270,10 +365,7 @@ var _ = Describe("loadbalancer controller", Serial, Ordered, func() {
 
 			Eventually(func(g Gomega) {
 				var lbsetList yawolv1beta1.LoadBalancerSetList
-				g.Expect(k8sClient.List(ctx, &lbsetList, &runtimeClient.ListOptions{
-					LabelSelector: labels.SelectorFromSet(lb.Spec.Selector.MatchLabels),
-				})).Should(Succeed())
-
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
 				g.Expect(len(lbsetList.Items)).Should(Equal(1))
 
 				lbset := lbsetList.Items[0]
@@ -287,10 +379,7 @@ var _ = Describe("loadbalancer controller", Serial, Ordered, func() {
 
 			Eventually(func(g Gomega) {
 				var lbsetList yawolv1beta1.LoadBalancerSetList
-				g.Expect(k8sClient.List(ctx, &lbsetList, &runtimeClient.ListOptions{
-					LabelSelector: labels.SelectorFromSet(lb.Spec.Selector.MatchLabels),
-				})).Should(Succeed())
-
+				g.Expect(k8sClient.List(ctx, &lbsetList, runtimeClient.MatchingLabels(lb.Spec.Selector.MatchLabels))).Should(Succeed())
 				g.Expect(len(lbsetList.Items)).Should(Equal(1))
 
 				lbset := lbsetList.Items[0]

controllers/yawol-controller/loadbalancer/loadbalancerset_status_controller.go

@@ -57,7 +57,7 @@ func (r *LoadBalancerSetStatusReconciler) Reconcile(ctx context.Context, req ctr
 	}
 
 	// Not the current revision
-	if lb.Annotations[RevisionAnnotation] != loadBalancerSet.Annotations[RevisionAnnotation] {
+	if lb.Annotations[helper.RevisionAnnotation] != loadBalancerSet.Annotations[helper.RevisionAnnotation] {
 		return ctrl.Result{}, nil
 	}