diff --git a/api/v2/config/controller.go b/api/v2/config/controller.go index 74094ba..f12a185 100644 --- a/api/v2/config/controller.go +++ b/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 } diff --git a/api/v2/types_firewall.go b/api/v2/types_firewall.go index e768905..34b18e8 100644 --- a/api/v2/types_firewall.go +++ b/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. diff --git a/api/v2/types_firewalldeployment.go b/api/v2/types_firewalldeployment.go index d7b8268..69e01fa 100644 --- a/api/v2/types_firewalldeployment.go +++ b/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. diff --git a/api/v2/zz_generated.deepcopy.go b/api/v2/zz_generated.deepcopy.go index b3cd8bc..b9d9399 100644 --- a/api/v2/zz_generated.deepcopy.go +++ b/api/v2/zz_generated.deepcopy.go @@ -208,6 +208,21 @@ func (in *Firewall) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallAutoUpdate) DeepCopyInto(out *FirewallAutoUpdate) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallAutoUpdate. +func (in *FirewallAutoUpdate) DeepCopy() *FirewallAutoUpdate { + if in == nil { + return nil + } + out := new(FirewallAutoUpdate) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FirewallDeployment) DeepCopyInto(out *FirewallDeployment) { *out = *in @@ -270,6 +285,7 @@ func (in *FirewallDeploymentList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FirewallDeploymentSpec) DeepCopyInto(out *FirewallDeploymentSpec) { *out = *in + out.AutoUpdate = in.AutoUpdate if in.Selector != nil { in, out := &in.Selector, &out.Selector *out = make(map[string]string, len(*in)) diff --git a/config/crds/firewall.metal-stack.io_firewalldeployments.yaml b/config/crds/firewall.metal-stack.io_firewalldeployments.yaml index af0ea84..3100ea4 100644 --- a/config/crds/firewall.metal-stack.io_firewalldeployments.yaml +++ b/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: diff --git a/config/crds/firewall.metal-stack.io_firewallmonitors.yaml b/config/crds/firewall.metal-stack.io_firewallmonitors.yaml index c73f36a..dc859c3 100644 --- a/config/crds/firewall.metal-stack.io_firewallmonitors.yaml +++ b/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. diff --git a/config/crds/firewall.metal-stack.io_firewalls.yaml b/config/crds/firewall.metal-stack.io_firewalls.yaml index f0f56f9..c82118b 100644 --- a/config/crds/firewall.metal-stack.io_firewalls.yaml +++ b/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. diff --git a/controllers/deployment/controller.go b/controllers/deployment/controller.go index a604540..6366b87 100644 --- a/controllers/deployment/controller.go +++ b/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 diff --git a/controllers/deployment/reconcile.go b/controllers/deployment/reconcile.go index 0b19889..c427791 100644 --- a/controllers/deployment/reconcile.go +++ b/controllers/deployment/reconcile.go @@ -1,7 +1,6 @@ package deployment import ( - "context" "fmt" "strconv" "time" @@ -9,8 +8,6 @@ import ( "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,10 +186,10 @@ 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 ( @@ -200,49 +197,20 @@ func (c *controller) isNewSetRequired(r *controllers.Ctx[*v2.FirewallDeployment] 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 } diff --git a/controllers/deployment/recreate.go b/controllers/deployment/recreate.go index 819a43a..b7b5322 100644 --- a/controllers/deployment/recreate.go +++ b/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 } diff --git a/controllers/deployment/rolling.go b/controllers/deployment/rolling.go index 1875002..5a5e08a 100644 --- a/controllers/deployment/rolling.go +++ b/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) } diff --git a/controllers/firewall/status.go b/controllers/firewall/status.go index 29c501d..693fbe8 100644 --- a/controllers/firewall/status.go +++ b/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 diff --git a/controllers/update/controller.go b/controllers/update/controller.go new file mode 100644 index 0000000..991adde --- /dev/null +++ b/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 + }) +} diff --git a/controllers/update/reconcile.go b/controllers/update/reconcile.go new file mode 100644 index 0000000..fa6f0b6 --- /dev/null +++ b/controllers/update/reconcile.go @@ -0,0 +1,146 @@ +package update + +import ( + "errors" + "fmt" + "strconv" + "strings" + + "github.com/Masterminds/semver/v3" + v2 "github.com/metal-stack/firewall-controller-manager/api/v2" + "github.com/metal-stack/firewall-controller-manager/controllers" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallDeployment]) error { + return c.autoUpdateOS(r) +} + +func (c *controller) autoUpdateOS(r *controllers.Ctx[*v2.FirewallDeployment]) error { + if !r.Target.Spec.AutoUpdate.MachineImage { + return nil + } + + if !r.WithinMaintenance { + c.log.Info("not checking for newer os image, not in maintenance time window") + return nil + } + + c.log.Info("checking for newer os image") + + // first, let's resolve the latest image from the api + + os, version, err := getOsAndSemverFromImage(r.Target.Spec.Template.Spec.Image) + if err != nil { + return fmt.Errorf("image version cannot be parsed: %w", err) + } + + var ( + imageToResolve = r.Target.Spec.Template.Spec.Image + isFullyQualifiedImageNotation = version.Patch() != 0 + ) + + if isFullyQualifiedImageNotation { + imageToResolve = fmt.Sprintf("%s-%d.%d", os, version.Major(), version.Minor()) + } + + image, err := c.imageCache.Get(r.Ctx, imageToResolve) + if err != nil { + return fmt.Errorf("unable to retrieve latest os image from metal-api: %w", err) + } + + if image.ID == nil { + return fmt.Errorf("returned image from metal-api contains no id") + } + + // now figure out the actual running image of the current firewall + // this can be different from specification in case a shorthand image notation is being used + // so we need the exact version running + + ownedSets, _, err := controllers.GetOwnedResources(r.Ctx, c.c.GetSeedClient(), nil, r.Target, &v2.FirewallSetList{}, func(fsl *v2.FirewallSetList) []*v2.FirewallSet { + return fsl.GetItems() + }) + if err != nil { + return fmt.Errorf("unable to get owned sets: %w", err) + } + + latestSet, err := controllers.MaxRevisionOf(ownedSets) + if err != nil { + return err + } + + ownedFirewalls, _, err := controllers.GetOwnedResources(r.Ctx, c.c.GetSeedClient(), nil, latestSet, &v2.FirewallList{}, func(fl *v2.FirewallList) []*v2.Firewall { + return fl.GetItems() + }) + if err != nil { + return fmt.Errorf("unable to get owned firewalls: %w", err) + } + + v2.SortFirewallsByImportance(ownedFirewalls) + + if len(ownedFirewalls) == 0 { + return nil + } + + fw := ownedFirewalls[0] // this is the currently active one + + if fw.Status.MachineStatus == nil || fw.Status.MachineStatus.ImageID == "" { + return nil + } + + // finally, we can do the image comparison + + if *image.ID == fw.Status.MachineStatus.ImageID { + r.Log.Info("no new os version available, not triggering auto-update") + return nil + } + + r.Log.Info("newer os version is available, triggering auto-update") + + err = retry.RetryOnConflict(retry.DefaultBackoff, func() error { + refetched := &v2.FirewallDeployment{} + err := c.c.GetSeedClient().Get(r.Ctx, client.ObjectKeyFromObject(r.Target), refetched) + if err != nil { + return fmt.Errorf("unable re-fetch firewall deployment: %w", err) + } + + if refetched.Annotations == nil { + refetched.Annotations = map[string]string{} + } + + refetched.Annotations[v2.RollSetAnnotation] = strconv.FormatBool(true) + if isFullyQualifiedImageNotation { + refetched.Spec.Template.Spec.Image = *image.ID + } + + err = c.c.GetSeedClient().Update(r.Ctx, refetched) + if err != nil { + return fmt.Errorf("unable to update firewall deployment: %w", err) + } + + return nil + }) + if err != nil { + return err + } + + return nil +} + +// copied over from metal-api because this is only available in internal package +func getOsAndSemverFromImage(id string) (string, *semver.Version, error) { + imageParts := strings.Split(id, "-") + if len(imageParts) < 2 { + return "", nil, errors.New("image does not contain a version") + } + + parts := len(imageParts) - 1 + os := strings.Join(imageParts[:parts], "-") + version := strings.Join(imageParts[parts:], "") + v, err := semver.NewVersion(version) + if err != nil { + return "", nil, err + } + return os, v, nil +} diff --git a/controllers/update/reconcile_test.go b/controllers/update/reconcile_test.go new file mode 100644 index 0000000..6ba56f8 --- /dev/null +++ b/controllers/update/reconcile_test.go @@ -0,0 +1,262 @@ +package update + +import ( + "context" + "strconv" + "testing" + + "github.com/go-logr/logr" + "github.com/google/go-cmp/cmp" + 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" + "github.com/metal-stack/metal-go/api/client/image" + "github.com/metal-stack/metal-go/api/models" + metaltestclient "github.com/metal-stack/metal-go/test/client" + "github.com/metal-stack/metal-lib/pkg/pointer" + "github.com/metal-stack/metal-lib/pkg/testcommon" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func Test_controller_autoUpdateOS(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + err := v2.AddToScheme(scheme) + require.NoError(t, err) + + tests := []struct { + name string + metalMocks *metaltestclient.MetalMockFns + fwDeploy *v2.FirewallDeployment + existingFws []v2.Firewall + postTestFn func(t *testing.T, c client.Client) + want bool + withinMaintenance bool + wantErr error + }{ + { + name: "auto-update disabled", + fwDeploy: &v2.FirewallDeployment{ + Spec: v2.FirewallDeploymentSpec{ + Template: v2.FirewallTemplateSpec{ + Spec: v2.FirewallSpec{ + Image: "a", + }, + }, + AutoUpdate: v2.FirewallAutoUpdate{ + MachineImage: false, + }, + }, + }, + withinMaintenance: true, + wantErr: nil, + }, + { + name: "not in maintenance time window", + fwDeploy: &v2.FirewallDeployment{ + Spec: v2.FirewallDeploymentSpec{ + Template: v2.FirewallTemplateSpec{ + Spec: v2.FirewallSpec{ + Image: "a", + }, + }, + AutoUpdate: v2.FirewallAutoUpdate{ + MachineImage: true, + }, + }, + }, + withinMaintenance: false, + wantErr: nil, + }, + { + name: "not in maintenance time window", + fwDeploy: &v2.FirewallDeployment{ + Spec: v2.FirewallDeploymentSpec{ + Template: v2.FirewallTemplateSpec{ + Spec: v2.FirewallSpec{ + Image: "a", + }, + }, + AutoUpdate: v2.FirewallAutoUpdate{ + MachineImage: true, + }, + }, + }, + withinMaintenance: false, + wantErr: nil, + }, + { + name: "auto-update when using shorthand image notation", + fwDeploy: &v2.FirewallDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment", + Namespace: "firewall", + }, + Spec: v2.FirewallDeploymentSpec{ + Template: v2.FirewallTemplateSpec{ + Spec: v2.FirewallSpec{ + Image: "firewall-ubuntu-3.0", + }, + }, + AutoUpdate: v2.FirewallAutoUpdate{ + MachineImage: true, + }, + }, + }, + withinMaintenance: true, + metalMocks: &metaltestclient.MetalMockFns{ + Image: func(mock *mock.Mock) { + mock.On("FindLatestImage", image.NewFindLatestImageParams().WithID("firewall-ubuntu-3.0").WithContext(ctx), nil).Return(&image.FindLatestImageOK{ + Payload: &models.V1ImageResponse{ + ID: pointer.Pointer("firewall-ubuntu-3.0.20240503"), + }, + }, nil) + }, + }, + existingFws: []v2.Firewall{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "firewall", + }, + Status: v2.FirewallStatus{ + MachineStatus: &v2.MachineStatus{ + ImageID: "firewall-ubuntu-3.0.20240101", + }, + }, + }, + }, + postTestFn: func(t *testing.T, c client.Client) { + fwdeploy := &v2.FirewallDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment", + Namespace: "firewall", + }, + } + err := c.Get(context.Background(), client.ObjectKeyFromObject(fwdeploy), fwdeploy) + require.NoError(t, err) + + assert.Equal(t, "firewall-ubuntu-3.0", fwdeploy.Spec.Template.Spec.Image) + assert.Equal(t, fwdeploy.Annotations[v2.RollSetAnnotation], strconv.FormatBool(true)) + }, + wantErr: nil, + }, + { + name: "auto-update when using fully-qualified image notation", + fwDeploy: &v2.FirewallDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment", + Namespace: "firewall", + }, + Spec: v2.FirewallDeploymentSpec{ + Template: v2.FirewallTemplateSpec{ + Spec: v2.FirewallSpec{ + Image: "firewall-ubuntu-3.0.20230101", + }, + }, + AutoUpdate: v2.FirewallAutoUpdate{ + MachineImage: true, + }, + }, + }, + withinMaintenance: true, + metalMocks: &metaltestclient.MetalMockFns{ + Image: func(mock *mock.Mock) { + mock.On("FindLatestImage", image.NewFindLatestImageParams().WithID("firewall-ubuntu-3.0").WithContext(ctx), nil).Return(&image.FindLatestImageOK{ + Payload: &models.V1ImageResponse{ + ID: pointer.Pointer("firewall-ubuntu-3.0.20240503"), + }, + }, nil) + }, + }, + existingFws: []v2.Firewall{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "firewall", + }, + Status: v2.FirewallStatus{ + MachineStatus: &v2.MachineStatus{ + ImageID: "firewall-ubuntu-3.0.20230101", + }, + }, + }, + }, + postTestFn: func(t *testing.T, c client.Client) { + fwdeploy := &v2.FirewallDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment", + Namespace: "firewall", + }, + } + err := c.Get(context.Background(), client.ObjectKeyFromObject(fwdeploy), fwdeploy) + require.NoError(t, err) + + assert.Equal(t, "firewall-ubuntu-3.0.20240503", fwdeploy.Spec.Template.Spec.Image) + assert.Equal(t, fwdeploy.Annotations[v2.RollSetAnnotation], strconv.FormatBool(true)) + }, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, mc := metaltestclient.NewMetalMockClient(t, tt.metalMocks) + + latestSet := v2.FirewallSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "firewall", + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(tt.fwDeploy, v2.GroupVersion.WithKind("FirewallDeployment")), + }, + }, + } + + var ownedFirewalls []v2.Firewall + for _, existingFw := range tt.existingFws { + ownedFirewall := existingFw.DeepCopy() + ownedFirewall.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ + *metav1.NewControllerRef(&latestSet, v2.GroupVersion.WithKind("FirewallSet")), + } + ownedFirewalls = append(ownedFirewalls, *ownedFirewall) + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithLists( + &v2.FirewallSetList{Items: []v2.FirewallSet{latestSet}}, + &v2.FirewallList{Items: ownedFirewalls}, + ).WithObjects(tt.fwDeploy).Build() + + cc, err := config.New(&config.NewControllerConfig{ + SeedClient: client, + SkipValidation: true, + }) + require.NoError(t, err) + + c := &controller{ + c: cc, + imageCache: newImageCache(mc), + } + + r := &controllers.Ctx[*v2.FirewallDeployment]{ + Ctx: ctx, + Log: logr.Logger{}, + Target: tt.fwDeploy, + WithinMaintenance: tt.withinMaintenance, + } + + err = c.autoUpdateOS(r) + if diff := cmp.Diff(tt.wantErr, err, testcommon.ErrorStringComparer()); diff != "" { + t.Errorf("error diff (+got -want):\n %s", diff) + } + + if tt.postTestFn != nil { + tt.postTestFn(t, client) + } + }) + } +} diff --git a/integration/integration_test.go b/integration/integration_test.go index 1a6637a..daf0296 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -19,6 +19,7 @@ import ( testcommon "github.com/metal-stack/firewall-controller-manager/integration/common" metalfirewall "github.com/metal-stack/metal-go/api/client/firewall" + "github.com/metal-stack/metal-go/api/client/image" "github.com/metal-stack/metal-go/api/client/machine" "github.com/metal-stack/metal-go/api/client/network" "github.com/metal-stack/metal-go/api/models" @@ -176,6 +177,9 @@ var _ = Context("integration test", Ordered, func() { Machine: func(m *mock.Mock) { m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Expect(k8sClient.Create(ctx, deployment())).To(Succeed()) @@ -447,6 +451,9 @@ var _ = Context("integration test", Ordered, func() { Machine: func(m *mock.Mock) { m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) deploy := deployment() @@ -739,6 +746,9 @@ var _ = Context("integration test", Ordered, func() { Network: func(m *mock.Mock) { m.On("FindNetwork", mock.Anything, nil).Return(&network.FindNetworkOK{Payload: network1}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(mon), mon)).To(Succeed()) // refetch @@ -827,6 +837,9 @@ var _ = Context("integration test", Ordered, func() { m.On("FreeMachine", mock.Anything, nil).Return(&machine.FreeMachineOK{Payload: &models.V1MachineResponse{ID: firewall1.ID}}, nil).Maybe() m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Expect(k8sClient.Delete(ctx, deployment())).To(Succeed()) @@ -874,6 +887,9 @@ var _ = Context("integration test", Ordered, func() { Machine: func(m *mock.Mock) { m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) deploy := deployment() @@ -1175,6 +1191,9 @@ var _ = Context("integration test", Ordered, func() { m.On("FreeMachine", mock.Anything, nil).Return(nil, &machine.FreeMachineDefault{Payload: httperrors.Conflict(fmt.Errorf("deletion blocked"))}).Maybe() m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) deploy.Spec.Template.Spec.Networks = []string{"internet", "mpls"} @@ -1211,6 +1230,9 @@ var _ = Context("integration test", Ordered, func() { m.On("FreeMachine", mock.Anything, nil).Return(&machine.FreeMachineOK{Payload: &models.V1MachineResponse{ID: firewall1.ID}}, nil).Maybe() m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Eventually(func() bool { @@ -1276,6 +1298,9 @@ var _ = Context("integration test", Ordered, func() { Network: func(m *mock.Mock) { m.On("FindNetwork", mock.Anything, nil).Return(&network.FindNetworkOK{Payload: network1}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(newMon), newMon)).To(Succeed()) // refetch @@ -1538,6 +1563,9 @@ var _ = Context("integration test", Ordered, func() { m.On("FreeMachine", mock.Anything, nil).Return(&machine.FreeMachineOK{Payload: &models.V1MachineResponse{ID: firewall1.ID}}, nil).Maybe() m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Expect(k8sClient.Delete(ctx, deployment())).To(Succeed()) @@ -1640,6 +1668,9 @@ var _ = Context("integration test", Ordered, func() { Machine: func(m *mock.Mock) { m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Expect(k8sClient.Create(ctx, fw)).To(Succeed()) @@ -1676,7 +1707,7 @@ var _ = Context("integration test", Ordered, func() { It("should adopt the existing firewall", func() { fw = testcommon.WaitForResourceAmount(k8sClient, ctx, namespaceName, 1, &v2.FirewallList{}, func(l *v2.FirewallList) []*v2.Firewall { return l.GetItems() - }, 3*time.Second) + }, 10*time.Second) Expect(fw.Name).To(Equal("test")) Expect(fw.OwnerReferences).To(HaveLen(1)) @@ -1846,6 +1877,9 @@ var _ = Context("integration test", Ordered, func() { m.On("FreeMachine", mock.Anything, nil).Return(&machine.FreeMachineOK{Payload: &models.V1MachineResponse{ID: firewall1.ID}}, nil).Maybe() m.On("UpdateMachine", mock.Anything, nil).Return(&machine.UpdateMachineOK{Payload: &models.V1MachineResponse{}}, nil).Maybe() }, + Image: func(m *mock.Mock) { + m.On("FindLatestImage", mock.Anything, nil).Return(&image.FindLatestImageOK{Payload: image1}, nil).Maybe() + }, }) Expect(k8sClient.Delete(ctx, deployment())).To(Succeed()) diff --git a/integration/metal_resources_test.go b/integration/metal_resources_test.go index c314dc6..01277df 100644 --- a/integration/metal_resources_test.go +++ b/integration/metal_resources_test.go @@ -247,13 +247,12 @@ var ( imageExpiration = pointer.Pointer(strfmt.DateTime(testTime.Add(3 * 24 * time.Hour))) image1 = &models.V1ImageResponse{ Classification: "supported", - Description: "debian-description", + Description: "firewall-image-description", ExpirationDate: imageExpiration, - Features: []string{"machine"}, - ID: pointer.Pointer("debian"), - Name: "debian-name", - URL: "debian-url", - Usedby: []string{"456"}, + Features: []string{"firewall"}, + ID: pointer.Pointer("firewall-ubuntu-2.0"), + Name: "firewall-image-name", + URL: "firewall-image-url", } partition1 = &models.V1PartitionResponse{ Bootconfig: &models.V1PartitionBootConfiguration{ diff --git a/integration/suite_test.go b/integration/suite_test.go index 89389c0..608e49f 100644 --- a/integration/suite_test.go +++ b/integration/suite_test.go @@ -16,6 +16,7 @@ import ( "github.com/metal-stack/firewall-controller-manager/controllers/firewall" "github.com/metal-stack/firewall-controller-manager/controllers/monitor" "github.com/metal-stack/firewall-controller-manager/controllers/set" + "github.com/metal-stack/firewall-controller-manager/controllers/update" metalclient "github.com/metal-stack/metal-go/test/client" "github.com/metal-stack/metal-lib/pkg/tag" . "github.com/onsi/ginkgo/v2" @@ -158,6 +159,14 @@ var _ = BeforeSuite(func() { ) Expect(err).ToNot(HaveOccurred()) + err = update.SetupWithManager( + ctrl.Log.WithName("controllers").WithName("update"), + mgr.GetEventRecorderFor("update-controller"), + mgr, + cc, + ) + Expect(err).ToNot(HaveOccurred()) + err = deployment.SetupWebhookWithManager(ctrl.Log.WithName("defaulting-webhook"), mgr, cc) Expect(err).ToNot(HaveOccurred()) err = set.SetupWebhookWithManager(ctrl.Log.WithName("defaulting-webhook"), mgr, cc) diff --git a/main.go b/main.go index e6a2893..2c6bad6 100644 --- a/main.go +++ b/main.go @@ -33,6 +33,7 @@ import ( "github.com/metal-stack/firewall-controller-manager/controllers/firewall" "github.com/metal-stack/firewall-controller-manager/controllers/monitor" "github.com/metal-stack/firewall-controller-manager/controllers/set" + "github.com/metal-stack/firewall-controller-manager/controllers/update" ) const ( @@ -41,7 +42,7 @@ const ( func healthCheckFunc(log *slog.Logger, seedClient controllerclient.Client, namespace string) func(req *http.Request) error { return func(req *http.Request) error { - log.Info("health check called") + log.Debug("health check called") fws := &v2.FirewallList{} err := seedClient.List(req.Context(), fws, controllerclient.InNamespace(namespace)) @@ -135,7 +136,7 @@ func main() { Cache: cache.Options{ SyncPeriod: &reconcileInterval, DefaultNamespaces: map[string]cache.Config{ - namespace: cache.Config{}, + namespace: {}, }, }, HealthProbeBindAddress: healthAddr, @@ -238,7 +239,7 @@ func main() { LeaderElection: false, Cache: cache.Options{ DefaultNamespaces: map[string]cache.Config{ - v2.FirewallShootNamespace: cache.Config{}, + v2.FirewallShootNamespace: {}, }, }, GracefulShutdownTimeout: pointer.Pointer(time.Duration(0)), @@ -272,16 +273,19 @@ func main() { } if err := deployment.SetupWithManager(ctrl.Log.WithName("controllers").WithName("deployment"), seedMgr.GetEventRecorderFor("firewall-deployment-controller"), seedMgr, cc); err != nil { - log.Fatalf("unable to setup controller deployment %v", err) + log.Fatalf("unable to setup deployment controller: %v", err) } if err := set.SetupWithManager(ctrl.Log.WithName("controllers").WithName("set"), seedMgr.GetEventRecorderFor("firewall-set-controller"), seedMgr, cc); err != nil { - log.Fatalf("unable to setup controller set %v", err) + log.Fatalf("unable to setup set controller: %v", err) } if err := firewall.SetupWithManager(ctrl.Log.WithName("controllers").WithName("firewall"), seedMgr.GetEventRecorderFor("firewall-controller"), seedMgr, cc); err != nil { - log.Fatalf("unable to setup controller firewall %v", err) + log.Fatalf("unable to setup firewall controller: %v", err) } if err := monitor.SetupWithManager(ctrl.Log.WithName("controllers").WithName("firewall-monitor"), shootMgr, cc); err != nil { - log.Fatalf("unable to setup controller monitor %v", err) + log.Fatalf("unable to setup monitor controller: %v", err) + } + if err := update.SetupWithManager(ctrl.Log.WithName("controllers").WithName("update"), seedMgr.GetEventRecorderFor("update-controller"), seedMgr, cc); err != nil { + log.Fatalf("unable to setup update controller: %v", err) } if err := deployment.SetupWebhookWithManager(ctrl.Log.WithName("defaulting-webhook"), seedMgr, cc); err != nil {