diff --git a/api/v2/types_firewall.go b/api/v2/types_firewall.go index bee2476..db324aa 100644 --- a/api/v2/types_firewall.go +++ b/api/v2/types_firewall.go @@ -36,6 +36,7 @@ const ( // +kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase" // +kubebuilder:printcolumn:name="Machine ID",type="string",JSONPath=".status.machineStatus.machineID" // +kubebuilder:printcolumn:name="Last Event",type="string",JSONPath=".status.machineStatus.lastEvent.event" +// +kubebuilder:printcolumn:name="Distance",type="string",priority=1,JSONPath=".distance" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".status.machineStatus.allocationTimestamp" // +kubebuilder:printcolumn:name="Spec Version",type="string",priority=1,JSONPath=".spec.controllerVersion" // +kubebuilder:printcolumn:name="Actual Version",type="string",priority=1,JSONPath=".status.controllerStatus.actualVersion" @@ -47,6 +48,10 @@ type Firewall struct { Spec FirewallSpec `json:"spec"` // Status contains current status information on the firewall. Status FirewallStatus `json:"status,omitempty"` + + // Distance defines the as-path length of a firewall. + // This field is typically orchestrated by the deployment controller. + Distance FirewallDistance `json:"distance"` } // FirewallSpec defines parameters for the firewall creation along with configuration for the firewall-controller. @@ -100,6 +105,7 @@ type FirewallSpec struct { // LogAcceptedConnections if set to true, also log accepted connections in the droptailer log. LogAcceptedConnections bool `json:"logAcceptedConnections,omitempty"` + // DNSServerAddress specifies DNS server address used by DNS proxy DNSServerAddress string `json:"dnsServerAddress,omitempty"` // DNSPort specifies port to which DNS proxy should be bound @@ -172,6 +178,8 @@ const ( FirewallControllerConnected ConditionType = "Connected" // FirewallMonitorDeployed indicates that the firewall monitor is deployed into the shoot cluster FirewallMonitorDeployed ConditionType = "MonitorDeployed" + // FirewallDistanceConfigured indicates that the firewall-controller has configured the given firewall distance. + FirewallDistanceConfigured ConditionType = "Distance" ) // ShootAccess contains secret references to construct a shoot client in the firewall-controller to update its firewall monitor. @@ -222,6 +230,8 @@ type ControllerConnection struct { ActualVersion string `json:"actualVersion,omitempty"` // Updated is a timestamp when the controller has last reconciled the firewall resource. Updated metav1.Time `json:"lastRun,omitempty"` + // ActualDistance is the actual distance as reflected by the firewall-controller. + ActualDistance FirewallDistance `json:"actualDistance,omitempty"` } // FirewallNetwork holds refined information about a network that the firewall is connected to. @@ -264,37 +274,39 @@ func (f *FirewallList) GetItems() []*Firewall { return result } -// SortFirewallsDeletion sorts the given firewall slice for deletion. +// SortFirewallsByImportance sorts the given firewall slice by importance, +// e.g. for scale down. +// // It considers certain criteria which firewalls should be kept longest and // which one's can be deleted first. The precedence is: // -// - Weight annotation (defaults to 0 if no annotation is present) -// - Connected firewalls -// - Ready firewalls -// - Created Firealls -// - Younger Firewalls +// - Weight annotation (prefer higher weight, defaults to 0 if no annotation is present) +// - Firewall lifecycle phase (connected > ready > created, prefer shorter distance when equal) +// - Firewall age (prefer younger firewalls) // // The firewalls at the beginning of the slice should be kept as long as possible. // The firewalls at the end of the slice should be removed first. // // The firewalls can be popped off from the slice in a deletion loop. -func SortFirewallsDeletion(fws []*Firewall) { - weight := func(fw *Firewall) (weight int) { - a, ok := fw.Annotations[FirewallWeightAnnotation] - if !ok { - return - } +func SortFirewallsByImportance(fws []*Firewall) { + var ( + conditionTypes = []ConditionType{FirewallControllerConnected, FirewallReady, FirewallCreated} + + weight = func(fw *Firewall) (weight int) { + a, ok := fw.Annotations[FirewallWeightAnnotation] + if !ok { + return + } - parsed, err := strconv.ParseInt(a, 10, 32) - if err != nil { + parsed, err := strconv.ParseInt(a, 10, 32) + if err != nil { + return + } + + weight = int(parsed) return } - - weight = int(parsed) - return - } - - conditionTypes := []ConditionType{FirewallControllerConnected, FirewallReady, FirewallCreated} + ) sort.Slice(fws, func(i, j int) bool { a := fws[i] @@ -319,9 +331,18 @@ func SortFirewallsDeletion(fws []*Firewall) { if !aTrue && bTrue { return false } + if aTrue && bTrue { + // prefer shorter distances because these are potentially "active" + if a.Distance < b.Distance { + return true + } + if a.Distance > b.Distance { + return false + } + } } - // prefer younger firewalls + // prefer younger firewalls (these potentially run on a more up-to-date operating system image) return !a.CreationTimestamp.Before(&b.CreationTimestamp) }) } diff --git a/api/v2/types_firewall_test.go b/api/v2/types_firewall_test.go index b96c5e6..3823240 100644 --- a/api/v2/types_firewall_test.go +++ b/api/v2/types_firewall_test.go @@ -9,7 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestSortForScaleDown(t *testing.T) { +func Test_SortFirewallsByImportance(t *testing.T) { now := time.Now() tests := []struct { @@ -32,6 +32,9 @@ func TestSortForScaleDown(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{Name: "lowest weight", Annotations: map[string]string{FirewallWeightAnnotation: "-100"}}, }, + { + ObjectMeta: metav1.ObjectMeta{Name: "connected shortest distance", CreationTimestamp: metav1.NewTime(now.Add(-20 * time.Minute))}, Distance: FirewallShortestDistance, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, + }, { ObjectMeta: metav1.ObjectMeta{Name: "middle", CreationTimestamp: metav1.NewTime(now)}, }, @@ -39,7 +42,7 @@ func TestSortForScaleDown(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "created"}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallCreated, ConditionTrue, "Created", "")}}, }, { - ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, + ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, }, { ObjectMeta: metav1.ObjectMeta{Name: "ready younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, @@ -51,7 +54,7 @@ func TestSortForScaleDown(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "no information"}, }, { - ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, + ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, }, }, want: []*Firewall{ @@ -59,10 +62,13 @@ func TestSortForScaleDown(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "highest weight", Annotations: map[string]string{FirewallWeightAnnotation: "100"}}, }, { - ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, + ObjectMeta: metav1.ObjectMeta{Name: "connected shortest distance", CreationTimestamp: metav1.NewTime(now.Add(-20 * time.Minute))}, Distance: FirewallShortestDistance, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, }, { - ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, + ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, }, { ObjectMeta: metav1.ObjectMeta{Name: "ready younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallReady, ConditionTrue, "Ready", "")}}, @@ -94,7 +100,7 @@ func TestSortForScaleDown(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - SortFirewallsDeletion(tt.fws) + SortFirewallsByImportance(tt.fws) if diff := cmp.Diff(tt.want, tt.fws, cmpopts.IgnoreFields(Condition{}, "LastTransitionTime", "LastUpdateTime")); diff != "" { t.Errorf("diff (+got -want):\n %s", diff) diff --git a/api/v2/types_firewalldeployment.go b/api/v2/types_firewalldeployment.go index cab9e61..d7b8268 100644 --- a/api/v2/types_firewalldeployment.go +++ b/api/v2/types_firewalldeployment.go @@ -12,6 +12,7 @@ import ( // +kubebuilder:object:root=true // +kubebuilder:resource:shortName=fwdeploy // +kubebuilder:subresource:status +// +kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.readyReplicas // +kubebuilder:printcolumn:name="Replicas",type=integer,JSONPath=`.spec.replicas` // +kubebuilder:printcolumn:name="Ready",type=integer,JSONPath=`.status.readyReplicas` // +kubebuilder:printcolumn:name="Progressing",type=integer,JSONPath=`.status.progressingReplicas` diff --git a/api/v2/types_firewallmonitor.go b/api/v2/types_firewallmonitor.go index e96e129..88d6f91 100644 --- a/api/v2/types_firewallmonitor.go +++ b/api/v2/types_firewallmonitor.go @@ -53,11 +53,13 @@ type FirewallMonitor struct { } type ControllerStatus struct { - Message string `json:"message,omitempty"` - FirewallStats *FirewallStats `json:"stats,omitempty"` - ControllerVersion string `json:"controllerVersion,omitempty"` - NftablesExporterVersion string `json:"nftablesExporterVersion,omitempty"` - Updated metav1.Time `json:"lastRun,omitempty"` + Message string `json:"message,omitempty"` + FirewallStats *FirewallStats `json:"stats,omitempty"` + ControllerVersion string `json:"controllerVersion,omitempty"` + NftablesExporterVersion string `json:"nftablesExporterVersion,omitempty"` + Updated metav1.Time `json:"lastRun,omitempty"` + Distance FirewallDistance `json:"distance,omitempty"` + DistanceSupported bool `json:"distanceSupported,omitempty"` } // FirewallStats contains firewall statistics diff --git a/api/v2/types_firewallset.go b/api/v2/types_firewallset.go index db3a92b..b19a4a8 100644 --- a/api/v2/types_firewallset.go +++ b/api/v2/types_firewallset.go @@ -12,6 +12,14 @@ import ( const ( // FirewallControllerSetAnnotation is a tag added to the firewall entity indicating to which set a firewall belongs to. FirewallControllerSetAnnotation = "firewall.metal.stack.io/set" + + FirewallShortestDistance = FirewallDistance(0) + FirewallRollingUpdateSetDistance = FirewallDistance(3) + FirewallLongestDistance = FirewallDistance(8) + + // FirewallMaxReplicas defines the maximum amount of firewall replicas to be defined. + // It does not make sense to allow large values here as it wastes a lot of machines. + FirewallMaxReplicas = 4 ) func FirewallSetTag(setName string) string { @@ -27,10 +35,12 @@ func FirewallManagedByTag() string { // +kubebuilder:object:root=true // +kubebuilder:resource:shortName=fwset // +kubebuilder:subresource:status +// +kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.readyReplicas // +kubebuilder:printcolumn:name="Replicas",type=integer,JSONPath=`.spec.replicas` // +kubebuilder:printcolumn:name="Ready",type=integer,JSONPath=`.status.readyReplicas` // +kubebuilder:printcolumn:name="Progressing",type=integer,JSONPath=`.status.progressingReplicas` // +kubebuilder:printcolumn:name="Unhealthy",type=integer,JSONPath=`.status.unhealthyReplicas` +// +kubebuilder:printcolumn:name="Distance",type="string",priority=1,JSONPath=".spec.distance" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" type FirewallSet struct { metav1.TypeMeta `json:",inline"` @@ -53,8 +63,24 @@ type FirewallSetSpec struct { Selector map[string]string `json:"selector,omitempty"` // Template is the firewall spec used for creating the firewalls. Template FirewallTemplateSpec `json:"template"` + // Distance defines the as-path length of the firewalls. + // This field is typically orchestrated by the deployment controller. + Distance FirewallDistance `json:"distance"` } +// FirewallDistance defines the as-path length of firewalls, influencing how strong they attract +// network traffic for routing traffic in and out of the cluster. +// This is of particular interest during rolling firewall updates, i.e. when there is +// more than a single firewall running in front of the cluster. +// During a rolling update, new firewalls start with a longer distance such that +// traffic is only attracted by the existing firewalls ("firewall staging"). +// When the new firewall has connected successfully to the firewall monitor, the deployment +// controller throws away the old firewalls and the new firewall takes over the routing. +// The deployment controller will then shorten the distance of the new firewall. +// This approach reduces service interruption of the external user traffic of the cluster +// (for firewall-controller versions that support this feature). +type FirewallDistance uint8 + type FirewallSetStatus struct { // TargetReplicas is the amount of firewall replicas targeted to be running. TargetReplicas int `json:"targetReplicas"` diff --git a/api/v2/validation/firewall.go b/api/v2/validation/firewall.go index 00dd6e0..eb43789 100644 --- a/api/v2/validation/firewall.go +++ b/api/v2/validation/firewall.go @@ -24,6 +24,7 @@ func (v *firewallValidator) ValidateCreate(log logr.Logger, f *v2.Firewall) fiel allErrs = append(allErrs, validateFirewallAnnotations(f)...) allErrs = append(allErrs, v.validateSpec(&f.Spec, field.NewPath("spec"))...) + allErrs = append(allErrs, validateDistance(f.Distance, field.NewPath("distance"))...) return allErrs } @@ -102,6 +103,7 @@ func (v *firewallValidator) ValidateUpdate(log logr.Logger, fOld, fNew *v2.Firew allErrs = append(allErrs, validateFirewallAnnotations(fNew)...) allErrs = append(allErrs, v.validateSpecUpdate(&fOld.Spec, &fNew.Spec, field.NewPath("spec"))...) + allErrs = append(allErrs, validateDistance(fNew.Distance, field.NewPath("distance"))...) return allErrs } @@ -136,3 +138,13 @@ func validateFirewallAnnotations(f *v2.Firewall) field.ErrorList { return allErrs } + +func validateDistance(distance v2.FirewallDistance, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if distance < v2.FirewallShortestDistance || distance > v2.FirewallLongestDistance { + allErrs = append(allErrs, field.Invalid(fldPath, distance, fmt.Sprintf("distance must be between %d and %d", v2.FirewallShortestDistance, v2.FirewallLongestDistance))) + } + + return allErrs +} diff --git a/api/v2/validation/firewall_test.go b/api/v2/validation/firewall_test.go index 621d709..22ce425 100644 --- a/api/v2/validation/firewall_test.go +++ b/api/v2/validation/firewall_test.go @@ -47,6 +47,7 @@ func Test_firewallValidator_ValidateCreate(t *testing.T) { }, }, }, + Distance: 0, } tests := []struct { @@ -141,6 +142,7 @@ func Test_firewallValidator_ValidateUpdate(t *testing.T) { }, }, }, + Distance: 0, }, oldF: &v2.Firewall{ ObjectMeta: metav1.ObjectMeta{ @@ -171,6 +173,7 @@ func Test_firewallValidator_ValidateUpdate(t *testing.T) { }, }, }, + Distance: 0, }, wantErr: nil, }, diff --git a/api/v2/validation/firewalldeployment.go b/api/v2/validation/firewalldeployment.go index 00f0c2a..ab19882 100644 --- a/api/v2/validation/firewalldeployment.go +++ b/api/v2/validation/firewalldeployment.go @@ -37,8 +37,8 @@ func (*firewallDeploymentValidator) validateSpec(log logr.Logger, f *v2.Firewall if f.Replicas < 0 { allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, "replicas cannot be a negative number")) } - if f.Replicas > 1 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, "for now, no more than a single firewall replica is allowed")) + if f.Replicas > v2.FirewallMaxReplicas { + allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, fmt.Sprintf("no more than %d firewall replicas are allowed", v2.FirewallMaxReplicas))) } if f.Selector == nil { diff --git a/api/v2/validation/firewallset.go b/api/v2/validation/firewallset.go index 785e7ba..821f10b 100644 --- a/api/v2/validation/firewallset.go +++ b/api/v2/validation/firewallset.go @@ -1,6 +1,8 @@ package validation import ( + "fmt" + "github.com/go-logr/logr" v2 "github.com/metal-stack/firewall-controller-manager/api/v2" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -29,6 +31,11 @@ func (v *firewallSetValidator) validateSpec(log logr.Logger, f *v2.FirewallSetSp if f.Replicas < 0 { allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, "replicas cannot be a negative number")) } + if f.Replicas > v2.FirewallMaxReplicas { + allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, fmt.Sprintf("no more than %d firewall replicas are allowed", v2.FirewallMaxReplicas))) + } + + allErrs = append(allErrs, validateDistance(f.Distance, fldPath.Child("distance"))...) if f.Selector == nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), f.Selector, "selector should not be nil")) diff --git a/api/v2/validation/firewallset_test.go b/api/v2/validation/firewallset_test.go index 6593b5b..14c904e 100644 --- a/api/v2/validation/firewallset_test.go +++ b/api/v2/validation/firewallset_test.go @@ -23,6 +23,7 @@ func Test_firewalSetValidator_ValidateCreate(t *testing.T) { Selector: map[string]string{ "purpose": "shoot-firewall", }, + Distance: 0, Template: v2.FirewallTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ @@ -109,6 +110,18 @@ func Test_firewalSetValidator_ValidateCreate(t *testing.T) { }, }, }, + { + name: "no more than max allowed replicas", + mutateFn: func(f *v2.FirewallSet) *v2.FirewallSet { + f.Spec.Replicas = 200 + return f + }, + wantErr: &apierrors.StatusError{ + ErrStatus: metav1.Status{ + Message: ` "firewall" is invalid: spec.replicas: Invalid value: 200: no more than 4 firewall replicas are allowed`, + }, + }, + }, } for _, tt := range tests { tt := tt @@ -134,6 +147,7 @@ func Test_firewallSetValidator_ValidateUpdate(t *testing.T) { "purpose": "shoot-firewall", "a": "b", }, + Distance: 0, Template: v2.FirewallTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ diff --git a/config/crds/firewall.metal-stack.io_firewalldeployments.yaml b/config/crds/firewall.metal-stack.io_firewalldeployments.yaml index 4a7c38b..584cf0f 100644 --- a/config/crds/firewall.metal-stack.io_firewalldeployments.yaml +++ b/config/crds/firewall.metal-stack.io_firewalldeployments.yaml @@ -316,4 +316,7 @@ spec: served: true storage: true subresources: + scale: + specReplicasPath: .spec.replicas + statusReplicasPath: .status.readyReplicas status: {} diff --git a/config/crds/firewall.metal-stack.io_firewallmonitors.yaml b/config/crds/firewall.metal-stack.io_firewallmonitors.yaml index 75ecbae..f0d94e6 100644 --- a/config/crds/firewall.metal-stack.io_firewallmonitors.yaml +++ b/config/crds/firewall.metal-stack.io_firewallmonitors.yaml @@ -88,6 +88,23 @@ spec: properties: controllerVersion: type: string + distance: + description: FirewallDistance defines the as-path length of firewalls, + influencing how strong they attract network traffic for routing + traffic in and out of the cluster. This is of particular interest + during rolling firewall updates, i.e. when there is more than a + single firewall running in front of the cluster. During a rolling + update, new firewalls start with a longer distance such that traffic + is only attracted by the existing firewalls ("firewall staging"). + When the new firewall has connected successfully to the firewall + monitor, the deployment controller throws away the old firewalls + and the new firewall takes over the routing. The deployment controller + will then shorten the distance of the new firewall. This approach + reduces service interruption of the external user traffic of the + cluster (for firewall-controller versions that support this feature). + type: integer + distanceSupported: + type: boolean lastRun: format: date-time type: string diff --git a/config/crds/firewall.metal-stack.io_firewalls.yaml b/config/crds/firewall.metal-stack.io_firewalls.yaml index 33c09e3..05d1fd2 100644 --- a/config/crds/firewall.metal-stack.io_firewalls.yaml +++ b/config/crds/firewall.metal-stack.io_firewalls.yaml @@ -27,6 +27,10 @@ spec: - jsonPath: .status.machineStatus.lastEvent.event name: Last Event type: string + - jsonPath: .distance + name: Distance + priority: 1 + type: string - jsonPath: .status.machineStatus.allocationTimestamp name: Age type: date @@ -49,6 +53,10 @@ spec: of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' type: string + distance: + description: Distance defines the as-path length of a firewall. This field + is typically orchestrated by the deployment controller. + type: integer kind: description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client @@ -238,6 +246,10 @@ spec: inside the firewall resource. This will be updated by the firewall monitor controller. properties: + actualDistance: + description: ActualDistance is the actual distance as reflected + by the firewall-controller. + type: integer actualVersion: description: ActualVersion is the actual version running at the firewall-controller. @@ -389,6 +401,7 @@ spec: - phase type: object required: + - distance - spec type: object served: true diff --git a/config/crds/firewall.metal-stack.io_firewallsets.yaml b/config/crds/firewall.metal-stack.io_firewallsets.yaml index 42fd0dc..bb817d4 100644 --- a/config/crds/firewall.metal-stack.io_firewallsets.yaml +++ b/config/crds/firewall.metal-stack.io_firewallsets.yaml @@ -30,6 +30,10 @@ spec: - jsonPath: .status.unhealthyReplicas name: Unhealthy type: integer + - jsonPath: .spec.distance + name: Distance + priority: 1 + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -55,6 +59,10 @@ spec: spec: description: Spec contains the firewall set specification. properties: + distance: + description: Distance defines the as-path length of the firewalls. + This field is typically orchestrated by the deployment controller. + type: integer replicas: description: Replicas is the amount of firewall replicas targeted to be running. @@ -235,6 +243,7 @@ spec: type: object type: object required: + - distance - replicas - template type: object @@ -273,4 +282,7 @@ spec: served: true storage: true subresources: + scale: + specReplicasPath: .spec.replicas + statusReplicasPath: .status.readyReplicas status: {} diff --git a/controllers/deployment/reconcile.go b/controllers/deployment/reconcile.go index b7df7cf..2581c1a 100644 --- a/controllers/deployment/reconcile.go +++ b/controllers/deployment/reconcile.go @@ -37,7 +37,7 @@ func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallDeployment]) error if current == nil { r.Log.Info("no firewall set is present, creating a new one") - _, err := c.createFirewallSet(r, 0) + _, err := c.createFirewallSet(r, v2.FirewallShortestDistance, 0) if err != nil { return err } @@ -63,19 +63,31 @@ func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallDeployment]) error return err } + // we are done with the update, give the set the shortest distance if this is not already the case + if current.Status.ReadyReplicas == current.Spec.Replicas && current.Spec.Distance != v2.FirewallShortestDistance { + current.Spec.Distance = v2.FirewallShortestDistance + + err := c.c.GetSeedClient().Update(r.Ctx, current) + if err != nil { + return fmt.Errorf("unable to swap latest set distance to %d: %w", v2.FirewallShortestDistance, err) + } + + r.Log.Info("swapped latest set to shortest distance", "distance", v2.FirewallShortestDistance) + } + return nil } -func (c *controller) createNextFirewallSet(r *controllers.Ctx[*v2.FirewallDeployment], current *v2.FirewallSet) (*v2.FirewallSet, error) { +func (c *controller) createNextFirewallSet(r *controllers.Ctx[*v2.FirewallDeployment], current *v2.FirewallSet, distance v2.FirewallDistance) (*v2.FirewallSet, error) { revision, err := controllers.NextRevision(current) if err != nil { return nil, err } - return c.createFirewallSet(r, revision) + return c.createFirewallSet(r, distance, revision) } -func (c *controller) createFirewallSet(r *controllers.Ctx[*v2.FirewallDeployment], revision int) (*v2.FirewallSet, error) { +func (c *controller) createFirewallSet(r *controllers.Ctx[*v2.FirewallDeployment], distance v2.FirewallDistance, revision int) (*v2.FirewallSet, error) { if lastCreation, ok := c.lastSetCreation[r.Target.Name]; ok && time.Since(lastCreation) < c.c.GetSafetyBackoff() { // this is just for safety reasons to prevent mass-allocations r.Log.Info("backing off from firewall set creation as last creation is only seconds ago", "ago", time.Since(lastCreation).String()) @@ -104,6 +116,7 @@ func (c *controller) createFirewallSet(r *controllers.Ctx[*v2.FirewallDeployment Spec: v2.FirewallSetSpec{ Replicas: r.Target.Spec.Replicas, Template: r.Target.Spec.Template, + Distance: distance, }, } diff --git a/controllers/deployment/recreate.go b/controllers/deployment/recreate.go index 921fe64..c03c743 100644 --- a/controllers/deployment/recreate.go +++ b/controllers/deployment/recreate.go @@ -22,7 +22,7 @@ func (c *controller) recreateStrategy(r *controllers.Ctx[*v2.FirewallDeployment] return err } - newSet, err := c.createNextFirewallSet(r, current) + newSet, err := c.createNextFirewallSet(r, current, v2.FirewallShortestDistance) if err != nil { return err } diff --git a/controllers/deployment/rolling.go b/controllers/deployment/rolling.go index dcdcf23..49174c0 100644 --- a/controllers/deployment/rolling.go +++ b/controllers/deployment/rolling.go @@ -16,9 +16,9 @@ func (c *controller) rollingUpdateStrategy(r *controllers.Ctx[*v2.FirewallDeploy } if newSetRequired { - r.Log.Info("significant changes detected in the spec, creating new firewall set") + r.Log.Info("significant changes detected in the spec, creating new firewall set", "distance", v2.FirewallRollingUpdateSetDistance) - newSet, err := c.createNextFirewallSet(r, current) + newSet, err := c.createNextFirewallSet(r, current, v2.FirewallRollingUpdateSetDistance) if err != nil { return err } diff --git a/controllers/firewall/reconcile.go b/controllers/firewall/reconcile.go index 935e936..69e5571 100644 --- a/controllers/firewall/reconcile.go +++ b/controllers/firewall/reconcile.go @@ -15,22 +15,17 @@ import ( "k8s.io/apimachinery/pkg/util/sets" ) -// Reconciler must always return either a error or requeue to ensure that it detects if a firewall get lost etc. +// Reconciler must always return either an error or requeue to ensure that it detects if a firewall get lost etc. func (c *controller) Reconcile(r *controllers.Ctx[*v2.Firewall]) error { var f *models.V1FirewallResponse defer func() { - _, err := c.ensureFirewallMonitor(r) + mon, err := c.ensureFirewallMonitor(r) if err != nil { r.Log.Error(err, "unable to deploy firewall monitor") // not returning, we can still try to update the status } - if f == nil { - r.Log.Error(fmt.Errorf("not owning a firewall"), "controller is not owning a firewall") - return - } - - if err := c.setStatus(r, f); err != nil { + if err := c.setStatus(r, f, mon); err != nil { r.Log.Error(err, "unable to set firewall status") } }() diff --git a/controllers/firewall/status.go b/controllers/firewall/status.go index 33f1296..72871bc 100644 --- a/controllers/firewall/status.go +++ b/controllers/firewall/status.go @@ -1,7 +1,6 @@ package firewall import ( - "context" "errors" "fmt" "time" @@ -9,56 +8,59 @@ import ( v2 "github.com/metal-stack/firewall-controller-manager/api/v2" "github.com/metal-stack/firewall-controller-manager/controllers" "github.com/metal-stack/metal-go/api/models" - "github.com/metal-stack/metal-lib/pkg/cache" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func (c *controller) setStatus(r *controllers.Ctx[*v2.Firewall], m *models.V1FirewallResponse) error { +func (c *controller) setStatus(r *controllers.Ctx[*v2.Firewall], f *models.V1FirewallResponse, mon *v2.FirewallMonitor) error { var errs []error - machineStatus, err := getMachineStatus(m) - if err == nil { - r.Target.Status.MachineStatus = machineStatus - } else { + err := setMachineStatus(r.Target, f) + if err != nil { errs = append(errs, err) } - firewallNetworks, err := getFirewallNetworks(r.Ctx, c.networkCache, m) - if err == nil { - r.Target.Status.FirewallNetworks = firewallNetworks - } else { + err = c.setFirewallNetworks(r, f) + if err != nil { errs = append(errs, err) } - if v2.IsAnnotationTrue(r.Target, v2.FirewallNoControllerConnectionAnnotation) { - cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionTrue, "NotChecking", "Not checking controller connection due to firewall annotation.") - r.Target.Status.Conditions.Set(cond) - } else if r.Target.Status.ControllerStatus == nil { - cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "NotConnected", "Controller has not yet connected.") - r.Target.Status.Conditions.Set(cond) - } + SetFirewallStatus(r.Target, mon) r.Target.Status.ShootAccess = c.c.GetShootAccess() return errors.Join(errs...) } -func getMachineStatus(m *models.V1FirewallResponse) (*v2.MachineStatus, error) { - result := &v2.MachineStatus{} +func setMachineStatus(fw *v2.Firewall, f *models.V1FirewallResponse) error { + if f == nil { + return nil + } - if m.ID == nil || m.Allocation == nil || m.Allocation.Created == nil || m.Liveliness == nil { - return nil, fmt.Errorf("firewall entity is missing essential fields") + result, err := getMachineStatus(f) + if err != nil { + return err } - result.MachineID = *m.ID - result.AllocationTimestamp = metav1.NewTime(time.Time(*m.Allocation.Created)) - result.Liveliness = *m.Liveliness + fw.Status.MachineStatus = result + + return nil +} - if m.Events != nil && m.Events.CrashLoop != nil { - result.CrashLoop = *m.Events.CrashLoop +func getMachineStatus(f *models.V1FirewallResponse) (*v2.MachineStatus, error) { + if f.ID == nil || f.Allocation == nil || f.Allocation.Created == nil || f.Liveliness == nil { + return nil, fmt.Errorf("firewall entity from metal-api is missing essential fields") } - if m.Events != nil && len(m.Events.Log) > 0 && m.Events.Log[0].Event != nil { - log := m.Events.Log[0] + + result := &v2.MachineStatus{} + result.MachineID = *f.ID + result.AllocationTimestamp = metav1.NewTime(time.Time(*f.Allocation.Created)) + result.Liveliness = *f.Liveliness + + if f.Events != nil && f.Events.CrashLoop != nil { + result.CrashLoop = *f.Events.CrashLoop + } + if f.Events != nil && len(f.Events.Log) > 0 && f.Events.Log[0].Event != nil { + log := f.Events.Log[0] result.LastEvent = &v2.MachineLastEvent{ Event: *log.Event, @@ -70,7 +72,7 @@ func getMachineStatus(m *models.V1FirewallResponse) (*v2.MachineStatus, error) { return result, nil } -func getFirewallNetworks(ctx context.Context, cache *cache.Cache[string, *models.V1NetworkResponse], m *models.V1FirewallResponse) ([]v2.FirewallNetwork, error) { +func (c *controller) setFirewallNetworks(r *controllers.Ctx[*v2.Firewall], f *models.V1FirewallResponse) error { // check whether network prefixes were updated in metal-api // prefixes in the firewall machine allocation are just a snapshot when the firewall was created. // -> when changing prefixes in the referenced network the firewall does not know about any prefix changes. @@ -78,20 +80,25 @@ func getFirewallNetworks(ctx context.Context, cache *cache.Cache[string, *models // we replace the prefixes from the snapshot with the actual prefixes that are currently attached to the network. // this allows dynamic prefix reconfiguration of the firewall. - if m.Allocation == nil { - return nil, fmt.Errorf("firewall entity is missing essential fields") + if f == nil { + return nil + } + + if f.Allocation == nil { + return fmt.Errorf("firewall entity is missing essential fields") } var result []v2.FirewallNetwork - for _, n := range m.Allocation.Networks { + + for _, n := range f.Allocation.Networks { n := n if n.Networkid == nil { continue } - nw, err := cache.Get(ctx, *n.Networkid) + nw, err := c.networkCache.Get(r.Ctx, *n.Networkid) if err != nil { - return nil, err + return err } result = append(result, v2.FirewallNetwork{ @@ -106,5 +113,63 @@ func getFirewallNetworks(ctx context.Context, cache *cache.Cache[string, *models }) } - return result, nil + r.Target.Status.FirewallNetworks = result + + return nil +} + +func SetFirewallStatus(fw *v2.Firewall, mon *v2.FirewallMonitor) { + if v2.IsAnnotationTrue(fw, v2.FirewallNoControllerConnectionAnnotation) { + cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionTrue, "NotChecking", "Not checking controller connection due to firewall annotation.") + fw.Status.Conditions.Set(cond) + + cond = v2.NewCondition(v2.FirewallDistanceConfigured, v2.ConditionTrue, "NotChecking", "Not checking distance due to firewall annotation.") + fw.Status.Conditions.Set(cond) + + return + } + + if mon == nil { + return + } + + if mon.ControllerStatus == nil { + cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "NotConnected", "Controller has not yet connected.") + fw.Status.Conditions.Set(cond) + + cond = v2.NewCondition(v2.FirewallDistanceConfigured, v2.ConditionFalse, "NotConnected", "Controller has not yet connected.") + fw.Status.Conditions.Set(cond) + + return + } + + connection := &v2.ControllerConnection{ + ActualVersion: mon.ControllerStatus.ControllerVersion, + Updated: mon.ControllerStatus.Updated, + ActualDistance: mon.ControllerStatus.Distance, + } + + fw.Status.ControllerStatus = connection + + if connection.Updated.Time.IsZero() { + cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "NotConnected", "Controller has not yet connected.") + fw.Status.Conditions.Set(cond) + } else if time.Since(connection.Updated.Time) > 5*time.Minute { + cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "StoppedReconciling", fmt.Sprintf("Controller has stopped reconciling since %s.", connection.Updated.Time.String())) + fw.Status.Conditions.Set(cond) + } else { + cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionTrue, "Connected", fmt.Sprintf("Controller reconciled firewall at %s.", connection.Updated.Time.String())) + fw.Status.Conditions.Set(cond) + } + + if !mon.ControllerStatus.DistanceSupported { + cond := v2.NewCondition(v2.FirewallDistanceConfigured, v2.ConditionTrue, "NotChecking", "Controller does not support distance reconciliation.") + fw.Status.Conditions.Set(cond) + } else if fw.Distance == connection.ActualDistance { + cond := v2.NewCondition(v2.FirewallDistanceConfigured, v2.ConditionTrue, "Configured", fmt.Sprintf("Controller has configured the specified distance %d.", fw.Distance)) + fw.Status.Conditions.Set(cond) + } else { + cond := v2.NewCondition(v2.FirewallDistanceConfigured, v2.ConditionFalse, "NotConfigured", fmt.Sprintf("Controller has configured distance %d, but %d is specified.", connection.ActualDistance, fw.Distance)) + fw.Status.Conditions.Set(cond) + } } diff --git a/controllers/monitor/reconcile.go b/controllers/monitor/reconcile.go index 56e336f..2a81f63 100644 --- a/controllers/monitor/reconcile.go +++ b/controllers/monitor/reconcile.go @@ -9,6 +9,7 @@ import ( v2 "github.com/metal-stack/firewall-controller-manager/api/v2" "github.com/metal-stack/firewall-controller-manager/api/v2/helper" "github.com/metal-stack/firewall-controller-manager/controllers" + "github.com/metal-stack/firewall-controller-manager/controllers/firewall" "github.com/metal-stack/metal-lib/pkg/pointer" corev1 "k8s.io/api/core/v1" @@ -52,33 +53,7 @@ func (c *controller) updateFirewallStatus(r *controllers.Ctx[*v2.FirewallMonitor return nil, fmt.Errorf("associated firewall of monitor not found: %w", err) } - if v2.IsAnnotationTrue(fw, v2.FirewallNoControllerConnectionAnnotation) { - cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionTrue, "NotChecking", "Not checking controller connection due to firewall annotation.") - fw.Status.Conditions.Set(cond) - } else { - if r.Target.ControllerStatus == nil { - cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "NotConnected", "Controller has not yet connected.") - fw.Status.Conditions.Set(cond) - } else { - connection := &v2.ControllerConnection{ - ActualVersion: r.Target.ControllerStatus.ControllerVersion, - Updated: r.Target.ControllerStatus.Updated, - } - - fw.Status.ControllerStatus = connection - - if connection.Updated.Time.IsZero() { - cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "NotConnected", "Controller has not yet connected.") - fw.Status.Conditions.Set(cond) - } else if time.Since(connection.Updated.Time) > 5*time.Minute { - cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "StoppedReconciling", fmt.Sprintf("Controller has stopped reconciling since %s.", connection.Updated.Time.String())) - fw.Status.Conditions.Set(cond) - } else { - cond := v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionTrue, "Connected", fmt.Sprintf("Controller reconciled firewall at %s.", connection.Updated.Time.String())) - fw.Status.Conditions.Set(cond) - } - } - } + firewall.SetFirewallStatus(fw, r.Target) err = c.c.GetSeedClient().Status().Update(r.Ctx, fw) if err != nil { diff --git a/controllers/set/reconcile.go b/controllers/set/reconcile.go index f6c0c64..b9b2894 100644 --- a/controllers/set/reconcile.go +++ b/controllers/set/reconcile.go @@ -26,13 +26,48 @@ func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallSet]) error { ownedFirewalls = append(ownedFirewalls, adoptions...) - for _, fw := range ownedFirewalls { + // we sort for distance orchestration and for scale down deletion + // - the most important firewall will get the shortest distance to attract all the traffic + // - the least important at the end of the slice can be popped off for deletion on scale down + v2.SortFirewallsByImportance(ownedFirewalls) + + for i, fw := range ownedFirewalls { fw.Spec = r.Target.Spec.Template.Spec + // stagger firewall replicas to achieve active/standby behavior + // + // we give the most important firewall the shortest distance within a set + // this firewall attracts the traffic within the replica set + // + // the second most important firewall gets a longer distance + // such that it takes over the traffic in case the first replica dies + // (first-level backup) + // + // the rest of the firewalls get an even higher distance + // (third-level backup) + // one of them moves up on next set sync after deletion of the "active" replica + var distance v2.FirewallDistance + switch i { + case 0: + distance = r.Target.Spec.Distance + 0 + case 1: + distance = r.Target.Spec.Distance + 1 + default: + distance = r.Target.Spec.Distance + 2 + } + + if distance > v2.FirewallLongestDistance { + distance = v2.FirewallLongestDistance + } + + fw.Distance = distance + err := c.c.GetSeedClient().Update(r.Ctx, fw, &client.UpdateOptions{}) if err != nil { return fmt.Errorf("error updating firewall spec: %w", err) } + + r.Log.Info("updated/synced firewall", "name", fw.Name, "distance", distance) } currentAmount := len(ownedFirewalls) @@ -57,9 +92,6 @@ func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallSet]) error { if currentAmount > r.Target.Spec.Replicas { r.Log.Info("scale down", "current", currentAmount, "want", r.Target.Spec.Replicas) - // puts the least important at the end of the slice, we will then pop them off for deletion - v2.SortFirewallsDeletion(ownedFirewalls) - for i := r.Target.Spec.Replicas; i < currentAmount; i++ { var fw *v2.Firewall fw, ownedFirewalls = pop(ownedFirewalls) @@ -122,6 +154,7 @@ func (c *controller) createFirewall(r *controllers.Ctx[*v2.FirewallSet]) (*v2.Fi fw := &v2.Firewall{ ObjectMeta: *meta, Spec: r.Target.Spec.Template.Spec, + Distance: r.Target.Spec.Distance, } err = c.c.GetSeedClient().Create(r.Ctx, fw, &client.CreateOptions{}) diff --git a/controllers/set/status.go b/controllers/set/status.go index faa87b1..5efdf53 100644 --- a/controllers/set/status.go +++ b/controllers/set/status.go @@ -22,9 +22,10 @@ func (c *controller) setStatus(r *controllers.Ctx[*v2.FirewallSet], ownedFirewal created = pointer.SafeDeref(fw.Status.Conditions.Get(v2.FirewallCreated)).Status == v2.ConditionTrue ready = pointer.SafeDeref(fw.Status.Conditions.Get(v2.FirewallReady)).Status == v2.ConditionTrue connected = pointer.SafeDeref(fw.Status.Conditions.Get(v2.FirewallControllerConnected)).Status == v2.ConditionTrue + distance = pointer.SafeDeref(fw.Status.Conditions.Get(v2.FirewallDistanceConfigured)).Status == v2.ConditionTrue ) - if created && ready && connected { + if created && ready && connected && distance { r.Target.Status.ReadyReplicas++ continue } diff --git a/integration/integration_test.go b/integration/integration_test.go index 9e5a17e..7941200 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -230,7 +230,9 @@ var _ = Context("integration test", Ordered, func() { // simulating a firewall-controller updating the resource Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(mon), mon)).To(Succeed()) // refetch mon.ControllerStatus = &v2.ControllerStatus{ - Updated: metav1.NewTime(time.Now()), + Updated: metav1.NewTime(time.Now()), + Distance: v2.FirewallShortestDistance, + DistanceSupported: true, } Expect(k8sClient.Update(ctx, mon)).To(Succeed()) }) @@ -315,6 +317,17 @@ var _ = Context("integration test", Ordered, func() { Expect(cond.Reason).To(Equal("Connected")) Expect(cond.Message).To(Equal(fmt.Sprintf("Controller reconciled firewall at %s.", mon.ControllerStatus.Updated.Time.String()))) }) + + It("should have configured the distance", func() { + cond := testcommon.WaitForCondition(k8sClient, ctx, fw.DeepCopy(), func(fd *v2.Firewall) v2.Conditions { + return fd.Status.Conditions + }, v2.FirewallDistanceConfigured, v2.ConditionTrue, 15*time.Second) + + Expect(cond.LastTransitionTime).NotTo(BeZero()) + Expect(cond.LastUpdateTime).NotTo(BeZero()) + Expect(cond.Reason).To(Equal("Configured")) + Expect(cond.Message).To(Equal(fmt.Sprintf("Controller has configured the specified distance %d.", v2.FirewallShortestDistance))) + }) }) Context("the firewall set resource", func() { @@ -463,7 +476,7 @@ var _ = Context("integration test", Ordered, func() { }) }) - Context("the firewall resource", func() { + Context("the new firewall resource", func() { It("should be named after the namespace (it's the shoot name in the end)", func() { Expect(fw.Name).To(HavePrefix(namespaceName + "-firewall-")) }) @@ -521,6 +534,17 @@ var _ = Context("integration test", Ordered, func() { Expect(cond.Message).To(Equal(fmt.Sprintf("Firewall %q is not ready.", *installingFirewall.Allocation.Name))) }) + It("should not yet have a distance configured", func() { + cond := testcommon.WaitForCondition(k8sClient, ctx, fw.DeepCopy(), func(fd *v2.Firewall) v2.Conditions { + return fd.Status.Conditions + }, v2.FirewallDistanceConfigured, v2.ConditionFalse, 15*time.Second) + + Expect(cond.LastTransitionTime).NotTo(BeZero()) + Expect(cond.LastUpdateTime).NotTo(BeZero()) + Expect(cond.Reason).To(Equal("NotConnected")) + Expect(cond.Message).To(Equal("Controller has not yet connected.")) + }) + It("should have the monitor condition true", func() { cond := testcommon.WaitForCondition(k8sClient, ctx, fw.DeepCopy(), func(fd *v2.Firewall) v2.Conditions { return fd.Status.Conditions @@ -582,7 +606,7 @@ var _ = Context("integration test", Ordered, func() { }) }) - Context("the firewall set resource", func() { + Context("the new firewall set resource", func() { It("should be named after the deployment", func() { Expect(set.Name).To(HavePrefix(deployment.Name + "-")) }) @@ -595,12 +619,16 @@ var _ = Context("integration test", Ordered, func() { Expect(set.Spec.Replicas).To(Equal(1)) }) - It("should inherit the spec from the deployement", func() { + It("should inherit the spec from the deployment", func() { wantSpec := deployment.Spec.Template.Spec.DeepCopy() - wantSpec.Size = "n2-medium-x86" + wantSpec.Size = "n2-medium-x86" // this is the change that triggered the rolling update Expect(&set.Spec.Template.Spec).To(BeComparableTo(wantSpec)) }) + It("should start with a higher distance", func() { + Expect(set.Spec.Distance).To(Equal(v2.FirewallRollingUpdateSetDistance)) + }) + It("should have the deployment as an owner", func() { Expect(set.ObjectMeta.OwnerReferences).To(HaveLen(1)) Expect(set.ObjectMeta.OwnerReferences[0].Name).To(Equal(deployment.Name)) @@ -684,7 +712,7 @@ var _ = Context("integration test", Ordered, func() { readyFirewall = firewall2("Phoned Home", "is phoning home") ) - When("the firewall gets ready and the firewall-controller connects", func() { + When("the firewall gets ready and the firewall-controller connects", Ordered, func() { It("should allow an update of the firewall monitor", func() { swapMetalClient(&metalclient.MetalMockFns{ Machine: func(m *mock.Mock) { @@ -702,10 +730,23 @@ var _ = Context("integration test", Ordered, func() { Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(mon), mon)).To(Succeed()) // refetch // simulating a firewall-controller updating the resource mon.ControllerStatus = &v2.ControllerStatus{ - Updated: metav1.NewTime(time.Now()), + Updated: metav1.NewTime(time.Now()), + Distance: v2.FirewallRollingUpdateSetDistance, + DistanceSupported: true, } Expect(k8sClient.Update(ctx, mon)).To(Succeed()) }) + + It("the firewall should reflect the distance during a rolling update", func() { + cond := testcommon.WaitForCondition(k8sClient, ctx, fw.DeepCopy(), func(fd *v2.Firewall) v2.Conditions { + return fd.Status.Conditions + }, v2.FirewallDistanceConfigured, v2.ConditionTrue, 15*time.Second) + + Expect(cond.LastTransitionTime).NotTo(BeZero()) + Expect(cond.LastUpdateTime).NotTo(BeZero()) + Expect(cond.Reason).To(Equal("Configured")) + Expect(cond.Message).To(Equal(fmt.Sprintf("Controller has configured the specified distance %d.", v2.FirewallRollingUpdateSetDistance))) + }) }) Context("the old generation disappears", func() { @@ -737,6 +778,14 @@ var _ = Context("integration test", Ordered, func() { return fw.Status.ControllerStatus }, 15*time.Second, interval).Should(Not(BeNil()), "controller connection was not synced to firewall resource") }) + + It("the firewall set should be updated to shortest distance as the update has succeeded", func() { + Eventually(func() v2.FirewallDistance { + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(set), set)).To(Succeed()) + return set.Spec.Distance + }).Within(5 * time.Second).ProbeEvery(200 * time.Millisecond).Should(Equal(v2.FirewallShortestDistance)) + Expect(set.Spec.Distance).To(Equal(v2.FirewallShortestDistance)) + }) }) })