Update firewalls in maintenance reconciliation

api/v2/config/controller.go

@@ -59,6 +59,9 @@ type NewControllerConfig struct {
 	FirewallHealthTimeout time.Duration
 	// CreateTimeout is used in the firewall creation phase to recreate a firewall when it does not become ready.
 	CreateTimeout time.Duration
+
+	// SkipValidation skips configuration validation, use this only for testing purposes
+	SkipValidation bool
 }
 
 type ControllerConfig struct {
@@ -90,7 +93,7 @@ type ControllerConfig struct {
 }
 
 func New(c *NewControllerConfig) (*ControllerConfig, error) {
-	if err := c.validate(); err != nil {
+	if err := c.validate(); !c.SkipValidation && err != nil {
 		return nil, err
 	}
 

api/v2/types_firewall.go

@@ -213,6 +213,8 @@ type MachineStatus struct {
 	CrashLoop bool `json:"crashLoop,omitempty"`
 	// LastEvent contains the last provisioning event of the machine.
 	LastEvent *MachineLastEvent `json:"lastEvent,omitempty"`
+	// ImageID contains the used os image id of the firewall (the fully qualified version, no shorthand version).
+	ImageID string `json:"imageID,omitempty"`
 }
 
 // MachineLastEvent contains the last provisioning event of the machine.

api/v2/types_firewalldeployment.go

@@ -46,6 +46,8 @@ type FirewallDeploymentSpec struct {
 	// Replicas is the amount of firewall replicas targeted to be running.
 	// Defaults to 1.
 	Replicas int `json:"replicas,omitempty"`
+	// AutoUpdate defines the behavior for automatic updates.
+	AutoUpdate FirewallAutoUpdate `json:"autoUpdate"`
 	// Selector is a label query over firewalls that should match the replicas count.
 	// If selector is empty, it is defaulted to the labels present on the firewall template.
 	// Label keys and values that must match in order to be controlled by this replication
@@ -55,6 +57,12 @@ type FirewallDeploymentSpec struct {
 	Template FirewallTemplateSpec `json:"template"`
 }
 
+type FirewallAutoUpdate struct {
+	// MachineImage auto updates the os image of the firewall within the maintenance time window
+	// in case a newer version of the os is available.
+	MachineImage bool `json:"machineImage"`
+}
+
 // FirewallDeploymentStatus contains current status information on the firewall deployment.
 type FirewallDeploymentStatus struct {
 	// TargetReplicas is the amount of firewall replicas targeted to be running.

config/crds/firewall.metal-stack.io_firewalldeployments.yaml

@@ -59,6 +59,17 @@ spec:
           spec:
             description: Spec contains the firewall deployment specification.
             properties:
+              autoUpdate:
+                description: AutoUpdate defines the behavior for automatic updates.
+                properties:
+                  machineImage:
+                    description: |-
+                      MachineImage auto updates the os image of the firewall within the maintenance time window
+                      in case a newer version of the os is available.
+                    type: boolean
+                required:
+                - machineImage
+                type: object
               replicas:
                 description: |-
                   Replicas is the amount of firewall replicas targeted to be running.
@@ -258,6 +269,7 @@ spec:
                     type: object
                 type: object
             required:
+            - autoUpdate
             - template
             type: object
           status:

config/crds/firewall.metal-stack.io_firewallmonitors.yaml

@@ -232,6 +232,10 @@ spec:
                 description: CrashLoop can occur during provisioning of the firewall
                   causing the firewall not to get ready.
                 type: boolean
+              imageID:
+                description: ImageID contains the used os image id of the firewall
+                  (the fully qualified version, no shorthand version).
+                type: string
               lastEvent:
                 description: LastEvent contains the last provisioning event of the
                   machine.

config/crds/firewall.metal-stack.io_firewalls.yaml

@@ -350,6 +350,10 @@ spec:
                     description: CrashLoop can occur during provisioning of the firewall
                       causing the firewall not to get ready.
                     type: boolean
+                  imageID:
+                    description: ImageID contains the used os image id of the firewall
+                      (the fully qualified version, no shorthand version).
+                    type: string
                   lastEvent:
                     description: LastEvent contains the last provisioning event of
                       the machine.

controllers/deployment/controller.go

@@ -36,6 +36,7 @@ func SetupWithManager(log logr.Logger, recorder record.EventRecorder, mgr ctrl.M
 			&v2.FirewallDeployment{},
 			builder.WithPredicates(
 				predicate.And(
+					predicate.Not(v2.AnnotationAddedPredicate(v2.MaintenanceAnnotation)),
 					predicate.Not(v2.AnnotationRemovedPredicate(v2.MaintenanceAnnotation)),
 					predicate.Or(
 						predicate.GenerationChangedPredicate{}, // prevents reconcile on status sub resource update

controllers/deployment/reconcile.go

@@ -1,16 +1,13 @@
 package deployment
 
 import (
-	"context"
 	"fmt"
 	"strconv"
 	"time"
 
 	"github.com/google/uuid"
 	v2 "github.com/metal-stack/firewall-controller-manager/api/v2"
 	"github.com/metal-stack/firewall-controller-manager/controllers"
-	metalgo "github.com/metal-stack/metal-go"
-	"github.com/metal-stack/metal-go/api/client/image"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/util/retry"
@@ -189,60 +186,31 @@ func (c *controller) syncFirewallSet(r *controllers.Ctx[*v2.FirewallDeployment],
 	return nil
 }
 
-func (c *controller) isNewSetRequired(r *controllers.Ctx[*v2.FirewallDeployment], latestSet *v2.FirewallSet) (bool, error) {
+func (c *controller) isNewSetRequired(r *controllers.Ctx[*v2.FirewallDeployment], latestSet *v2.FirewallSet) bool {
 	if v2.IsAnnotationTrue(latestSet, v2.RollSetAnnotation) {
 		r.Log.Info("set roll initiated by annotation")
-		return true, nil
+		return true
 	}
 
 	var (
 		newS = &r.Target.Spec.Template.Spec
 		oldS = &latestSet.Spec.Template.Spec
 	)
 
-	ok := sizeHasChanged(newS, oldS)
-	if ok {
+	if newS.Size != oldS.Size {
 		r.Log.Info("firewall size has changed", "size", newS.Size)
-		return ok, nil
+		return true
 	}
 
-	ok, err := osImageHasChanged(r.Ctx, c.c.GetMetal(), newS, oldS)
-	if err != nil {
-		return false, err
-	}
-	if ok {
+	if newS.Image != oldS.Image {
 		r.Log.Info("firewall image has changed", "image", newS.Image)
-		return ok, nil
+		return true
 	}
 
-	ok = networksHaveChanged(newS, oldS)
-	if ok {
+	if !sets.NewString(oldS.Networks...).Equal(sets.NewString(newS.Networks...)) {
 		r.Log.Info("firewall networks have changed", "networks", newS.Networks)
-		return ok, nil
-	}
-
-	return false, nil
-}
-
-func sizeHasChanged(newS *v2.FirewallSpec, oldS *v2.FirewallSpec) bool {
-	return newS.Size != oldS.Size
-}
-
-func osImageHasChanged(ctx context.Context, m metalgo.Client, newS *v2.FirewallSpec, oldS *v2.FirewallSpec) (bool, error) {
-	if newS.Image != oldS.Image {
-		image, err := m.Image().FindLatestImage(image.NewFindLatestImageParams().WithID(newS.Image).WithContext(ctx), nil)
-		if err != nil {
-			return false, fmt.Errorf("latest firewall image not found:%s %w", newS.Image, err)
-		}
-
-		if image.Payload != nil && image.Payload.ID != nil && *image.Payload.ID != oldS.Image {
-			return true, nil
-		}
+		return true
 	}
 
-	return false, nil
-}
-
-func networksHaveChanged(newS *v2.FirewallSpec, oldS *v2.FirewallSpec) bool {
-	return !sets.NewString(oldS.Networks...).Equal(sets.NewString(newS.Networks...))
+	return false
 }

controllers/deployment/recreate.go

@@ -11,12 +11,7 @@ import (
 
 // recreateStrategy first deletes the existing firewall sets and then creates a new one
 func (c *controller) recreateStrategy(r *controllers.Ctx[*v2.FirewallDeployment], ownedSets []*v2.FirewallSet, latestSet *v2.FirewallSet) error {
-	newSetRequired, err := c.isNewSetRequired(r, latestSet)
-	if err != nil {
-		return err
-	}
-
-	if newSetRequired {
+	if c.isNewSetRequired(r, latestSet) {
 		r.Log.Info("significant changes detected in the spec, create new scaled down firewall set, then cleaning up old sets")
 
 		set, err := c.createNextFirewallSet(r, latestSet, &setOverrides{
@@ -31,7 +26,7 @@ func (c *controller) recreateStrategy(r *controllers.Ctx[*v2.FirewallDeployment]
 		latestSet = set
 	}
 
-	err = c.deleteFirewallSets(r, controllers.Except(ownedSets, latestSet)...)
+	err := c.deleteFirewallSets(r, controllers.Except(ownedSets, latestSet)...)
 	if err != nil {
 		return err
 	}

controllers/deployment/rolling.go

@@ -10,12 +10,7 @@ import (
 
 // rollingUpdateStrategy first creates a new set and deletes the old one's when the new one becomes ready
 func (c *controller) rollingUpdateStrategy(r *controllers.Ctx[*v2.FirewallDeployment], ownedSets []*v2.FirewallSet, latestSet *v2.FirewallSet) error {
-	newSetRequired, err := c.isNewSetRequired(r, latestSet)
-	if err != nil {
-		return err
-	}
-
-	if newSetRequired {
+	if c.isNewSetRequired(r, latestSet) {
 		r.Log.Info("significant changes detected in the spec, creating new firewall set", "distance", v2.FirewallRollingUpdateSetDistance)
 
 		newSet, err := c.createNextFirewallSet(r, latestSet, &setOverrides{
@@ -32,7 +27,7 @@ func (c *controller) rollingUpdateStrategy(r *controllers.Ctx[*v2.FirewallDeploy
 		return c.cleanupIntermediateSets(r, ownedSets)
 	}
 
-	err = c.syncFirewallSet(r, latestSet)
+	err := c.syncFirewallSet(r, latestSet)
 	if err != nil {
 		return fmt.Errorf("unable to update firewall set: %w", err)
 	}

controllers/firewall/status.go

@@ -8,6 +8,7 @@ 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/pointer"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -45,14 +46,16 @@ func setMachineStatus(fw *v2.Firewall, f *models.V1FirewallResponse) error {
 }
 
 func getMachineStatus(f *models.V1FirewallResponse) (*v2.MachineStatus, error) {
-	if f.ID == nil || f.Allocation == nil || f.Allocation.Created == nil || f.Liveliness == nil {
+	if f.ID == nil || f.Allocation == nil || f.Allocation.Created == nil || f.Liveliness == nil || f.Allocation.Image == nil {
 		return nil, fmt.Errorf("firewall entity from metal-api is missing essential fields")
 	}
 
-	result := &v2.MachineStatus{}
-	result.MachineID = *f.ID
-	result.AllocationTimestamp = metav1.NewTime(time.Time(*f.Allocation.Created))
-	result.Liveliness = *f.Liveliness
+	result := &v2.MachineStatus{
+		MachineID:           *f.ID,
+		AllocationTimestamp: metav1.NewTime(time.Time(*f.Allocation.Created)),
+		Liveliness:          *f.Liveliness,
+		ImageID:             pointer.SafeDeref(f.Allocation.Image.ID),
+	}
 
 	if f.Events != nil && f.Events.CrashLoop != nil {
 		result.CrashLoop = *f.Events.CrashLoop

controllers/update/controller.go

@@ -0,0 +1,68 @@
+package update
+
+import (
+	"context"
+	"time"
+
+	"github.com/go-logr/logr"
+	"k8s.io/client-go/tools/record"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
+
+	v2 "github.com/metal-stack/firewall-controller-manager/api/v2"
+	"github.com/metal-stack/firewall-controller-manager/api/v2/config"
+	"github.com/metal-stack/firewall-controller-manager/controllers"
+	metalgo "github.com/metal-stack/metal-go"
+	"github.com/metal-stack/metal-go/api/client/image"
+	"github.com/metal-stack/metal-go/api/models"
+	"github.com/metal-stack/metal-lib/pkg/cache"
+)
+
+type controller struct {
+	c          *config.ControllerConfig
+	log        logr.Logger
+	recorder   record.EventRecorder
+	imageCache *cache.Cache[string, *models.V1ImageResponse]
+}
+
+func SetupWithManager(log logr.Logger, recorder record.EventRecorder, mgr ctrl.Manager, c *config.ControllerConfig) error {
+	g := controllers.NewGenericController(log, c.GetSeedClient(), c.GetSeedNamespace(), &controller{
+		c:          c,
+		log:        log,
+		recorder:   recorder,
+		imageCache: newImageCache(c.GetMetal()),
+	}).WithoutStatus()
+
+	return ctrl.NewControllerManagedBy(mgr).
+		For(
+			&v2.FirewallDeployment{},
+			builder.WithPredicates(
+				v2.AnnotationAddedPredicate(v2.MaintenanceAnnotation),
+			),
+		).
+		Named("FirewallDeployment").
+		WithEventFilter(predicate.NewPredicateFuncs(controllers.SkipOtherNamespace(c.GetSeedNamespace()))).
+		Complete(g)
+}
+
+func (c *controller) New() *v2.FirewallDeployment {
+	return &v2.FirewallDeployment{}
+}
+
+func (c *controller) SetStatus(_ *v2.FirewallDeployment, _ *v2.FirewallDeployment) {}
+
+func (c *controller) Delete(_ *controllers.Ctx[*v2.FirewallDeployment]) error {
+	return nil
+}
+
+func newImageCache(m metalgo.Client) *cache.Cache[string, *models.V1ImageResponse] {
+	return cache.New(5*time.Minute, func(ctx context.Context, id string) (*models.V1ImageResponse, error) {
+		resp, err := m.Image().FindLatestImage(image.NewFindLatestImageParams().WithID(id).WithContext(ctx), nil)
+		if err != nil {
+			return nil, err
+		}
+
+		return resp.Payload, nil
+	})
+}