diff --git a/cmd/boskos/boskos.go b/cmd/boskos/boskos.go index a9b1fbda..4339b945 100644 --- a/cmd/boskos/boskos.go +++ b/cmd/boskos/boskos.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "context" "flag" "fmt" "net/http" @@ -28,6 +29,13 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" "k8s.io/test-infra/pkg/flagutil" "k8s.io/test-infra/prow/config" @@ -36,6 +44,7 @@ import ( "k8s.io/test-infra/prow/logrusutil" prowmetrics "k8s.io/test-infra/prow/metrics" "k8s.io/test-infra/prow/pjutil" + "sigs.k8s.io/boskos/common" "sigs.k8s.io/boskos/crds" "sigs.k8s.io/boskos/handlers" "sigs.k8s.io/boskos/metrics" @@ -49,9 +58,9 @@ const ( ) var ( - configPath = flag.String("config", "config.yaml", "Path to init resource file") - dynamicResourceUpdatePeriod = flag.Duration("dynamic-resource-update-period", defaultDynamicResourceUpdatePeriod, - "Period at which to update dynamic resources. Set to 0 to disable.") + configPath = flag.String("config", "config.yaml", "Path to init resource file") + _ = flag.Duration("dynamic-resource-update-period", defaultDynamicResourceUpdatePeriod, + "Legacy flag that does nothing but is kept for compatibility reasons") requestTTL = flag.Duration("request-ttl", defaultRequestTTL, "request TTL before losing priority in the queue") logLevel = flag.String("log-level", "info", fmt.Sprintf("Log level is one of %v.", logrus.AllLevels)) namespace = flag.String("namespace", corev1.NamespaceDefault, "namespace to install on") @@ -99,12 +108,28 @@ func main() { // main server with the main mux until we're ready health := pjutil.NewHealth() - client, err := kubeClientOptions.CacheBackedClient(*namespace, &crds.ResourceObject{}, &crds.DRLCObject{}) + cfg, err := kubeClientOptions.Cfg() if err != nil { - logrus.WithError(err).Fatal("unable to get client") + logrus.WithError(err).Fatal("Failed to get kubeconfig") } + cfg.QPS = 100 + cfg.Burst = 200 + mgr, err := manager.New(cfg, manager.Options{ + LeaderElection: false, + Namespace: *namespace, + MetricsBindAddress: "0", + }) + if err != nil { + logrus.WithError(err).Fatal("Failed to construct mgr.") + } + interrupts.Run(func(ctx context.Context) { + if err := mgr.Start(ctx.Done()); err != nil { + logrus.WithError(err).Fatal("Mgr failed.") + } + logrus.Info("Mgr finished gracefully.") + }) - storage := ranch.NewStorage(interrupts.Context(), client, *namespace) + storage := ranch.NewStorage(interrupts.Context(), mgr.GetClient(), *namespace) r, err := ranch.NewRanch(*configPath, storage, *requestTTL) if err != nil { @@ -119,23 +144,26 @@ func main() { // Viper defaults the configfile name to `config` and `SetConfigFile` only // has an effect when the configfile name is not an empty string, so we // just disable it entirely if there is no config. + configChangeEventChan := make(chan event.GenericEvent) if *configPath != "" { v := viper.New() v.SetConfigFile(*configPath) v.SetConfigType("yaml") v.WatchConfig() v.OnConfigChange(func(in fsnotify.Event) { - logrus.Infof("Updating Boskos Config") - if err := r.SyncConfig(*configPath); err != nil { - logrus.WithError(err).Errorf("Failed to update config") - } else { - logrus.Infof("Updated Boskos Config successfully") - } + logrus.Info("Boskos config file changed, updating config.") + configChangeEventChan <- event.GenericEvent{} }) } + syncConfig := func() error { + return r.SyncConfig(*configPath) + } + if err := addConfigSyncReconcilerToManager(mgr, syncConfig, configChangeEventChan); err != nil { + logrus.WithError(err).Fatal("Failed to set up config sync controller") + } + prometheus.MustRegister(metrics.NewResourcesCollector(r)) - r.StartDynamicResourceUpdater(*dynamicResourceUpdatePeriod) r.StartRequestGC(defaultRequestGCPeriod) logrus.Info("Start Service") @@ -144,3 +172,72 @@ func main() { // signal to the world that we're ready health.ServeReady() } + +type configSyncReconciler struct { + sync func() error +} + +func (r *configSyncReconciler) Reconcile(_ reconcile.Request) (reconcile.Result, error) { + err := r.sync() + if err != nil { + logrus.WithError(err).Error("Config sync failed") + } + return reconcile.Result{}, err +} + +func addConfigSyncReconcilerToManager(mgr manager.Manager, configSync func() error, configChangeEvent <-chan event.GenericEvent) error { + ctrl, err := controller.New("bokos_config_reconciler", mgr, controller.Options{ + // We reconcile the whole config, hence this is not safe to run concurrently + MaxConcurrentReconciles: 1, + Reconciler: &configSyncReconciler{ + sync: configSync, + }, + }) + if err != nil { + return fmt.Errorf("failed to construct controller: %w", err) + } + + if err := ctrl.Watch(&source.Kind{Type: &crds.ResourceObject{}}, constHandler(), resourceUpdatePredicate()); err != nil { + return fmt.Errorf("failed to watch boskos resources: %w", err) + } + if err := ctrl.Watch(&source.Kind{Type: &crds.DRLCObject{}}, constHandler()); err != nil { + return fmt.Errorf("failed to watch boskos dynamic resources: %w", err) + } + if err := ctrl.Watch(&source.Channel{Source: configChangeEvent}, constHandler()); err != nil { + return fmt.Errorf("failed to create source channel for config change event: %w", err) + } + if err := mgr.Add(ctrl); err != nil { + return fmt.Errorf("failed to add controller to manager: %w", err) + } + + return nil +} + +func constHandler() handler.EventHandler { + return &handler.EnqueueRequestsFromMapFunc{ + ToRequests: handler.ToRequestsFunc(func(handler.MapObject) []reconcile.Request { + return []reconcile.Request{{}} + }), + } +} + +// resourceUpdatePredicate prevents the config reconciler from reacting to resource update events +// except if: +// * The new status is tombstone, because then we have to delete is +// * The new owner is empty, because then we have to delete it if it got deleted from the config but +// was not deleted from the api to let the current owner finish its work. +func resourceUpdatePredicate() predicate.Predicate { + return predicate.Funcs{ + CreateFunc: func(_ event.CreateEvent) bool { return true }, + DeleteFunc: func(_ event.DeleteEvent) bool { return true }, + UpdateFunc: func(e event.UpdateEvent) bool { + resource, ok := e.ObjectNew.(*crds.ResourceObject) + if !ok { + panic(fmt.Sprintf("BUG: expected *crds.ResourceObject, got %T", e.ObjectNew)) + } + + return resource.Status.State == common.Tombstone || resource.Status.Owner == "" + }, + GenericFunc: func(_ event.GenericEvent) bool { return true }, + } +} diff --git a/ranch/ranch.go b/ranch/ranch.go index b8e2aad4..56fee5a4 100644 --- a/ranch/ranch.go +++ b/ranch/ranch.go @@ -91,12 +91,6 @@ func NewRanch(config string, s *Storage, ttl time.Duration) (*Ranch, error) { requestMgr: NewRequestManager(ttl), now: time.Now, } - if config != "" { - if err := newRanch.SyncConfig(config); err != nil { - return nil, err - } - logrus.Infof("Loaded Boskos configuration successfully") - } return newRanch, nil } @@ -410,25 +404,6 @@ func (r *Ranch) SyncConfig(configPath string) error { return r.Storage.SyncResources(config) } -// StartDynamicResourceUpdater starts a goroutine which periodically -// updates all dynamic resources. -func (r *Ranch) StartDynamicResourceUpdater(updatePeriod time.Duration) { - if updatePeriod == 0 { - return - } - go func() { - updateTick := time.NewTicker(updatePeriod).C - for { - select { - case <-updateTick: - if err := r.Storage.UpdateAllDynamicResources(); err != nil { - logrus.WithError(err).Error("UpdateAllDynamicResources failed") - } - } - } - }() -} - // StartRequestGC starts the GC of expired requests func (r *Ranch) StartRequestGC(gcPeriod time.Duration) { r.requestMgr.StartGC(gcPeriod) diff --git a/ranch/ranch_test.go b/ranch/ranch_test.go index dac9e2a1..a0f209a0 100644 --- a/ranch/ranch_test.go +++ b/ranch/ranch_test.go @@ -18,16 +18,17 @@ package ranch import ( "context" + "errors" "fmt" "reflect" "sort" "sync" + "sync/atomic" "testing" "time" - "errors" - "github.com/go-test/deep" + "github.com/google/go-cmp/cmp" "github.com/sirupsen/logrus" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1386,10 +1387,96 @@ func TestSyncResources(t *testing.T) { }, }}, }, + { + name: "Dynamic boskos config adopts pre-existing static resources", + config: &common.BoskosConfig{Resources: []common.ResourceEntry{{ + Type: "test-resource", + State: common.Free, + MinCount: 10, + MaxCount: 10, + }}}, + currentRes: []runtime.Object{ + newResource("test-resource-0", "test-resource", common.Free, "", startTime), + newResource("test-resource-1", "test-resource", common.Free, "", startTime), + newResource("test-resource-2", "test-resource", common.Free, "", startTime), + }, + expectedRes: &crds.ResourceObjectList{Items: []crds.ResourceObject{ + *newResource("test-resource-0", "test-resource", common.Free, "", startTime), + *newResource("test-resource-1", "test-resource", common.Free, "", startTime), + *newResource("test-resource-2", "test-resource", common.Free, "", startTime), + *newResource("new-dynamic-res-1", "test-resource", common.Free, "", fakeNow), + *newResource("new-dynamic-res-2", "test-resource", common.Free, "", fakeNow), + *newResource("new-dynamic-res-3", "test-resource", common.Free, "", fakeNow), + *newResource("new-dynamic-res-4", "test-resource", common.Free, "", fakeNow), + *newResource("new-dynamic-res-5", "test-resource", common.Free, "", fakeNow), + *newResource("new-dynamic-res-6", "test-resource", common.Free, "", fakeNow), + *newResource("new-dynamic-res-7", "test-resource", common.Free, "", fakeNow), + }}, + expectedLCs: &crds.DRLCObjectList{Items: []crds.DRLCObject{{ + ObjectMeta: metav1.ObjectMeta{Name: "test-resource"}, + Spec: crds.DRLCSpec{ + InitialState: common.Free, + MinCount: 10, + MaxCount: 10, + }, + }}}, + }, + { + name: "Dynamic config that adopted static resources gets changed back to static", + config: &common.BoskosConfig{Resources: []common.ResourceEntry{{ + Type: "test-resource", + State: common.Free, + Names: []string{ + "test-resource-0", + "test-resource-1", + "test-resource-2", + }, + }}}, + currentRes: []runtime.Object{ + newResource("test-resource-0", "test-resource", common.Free, "", startTime), + newResource("test-resource-1", "test-resource", common.Free, "", startTime), + newResource("test-resource-2", "test-resource", common.Free, "", startTime), + newResource("fd957edc-4148-49e8-af83-53d38bcd4e54", "test-resource", common.Free, "", startTime), + newResource("fd957edc-4148-49e8-af83-53d38bcd4e55", "test-resource", common.Free, "", startTime), + newResource("fd957edc-4148-49e8-af83-53d38bcd4e56", "test-resource", common.Free, "", startTime), + newResource("fd957edc-4148-49e8-af83-53d38bcd4e57", "test-resource", common.Free, "", startTime), + newResource("fd957edc-4148-49e8-af83-53d38bcd4e58", "test-resource", common.Free, "", startTime), + newResource("fd957edc-4148-49e8-af83-53d38bcd4e59", "test-resource", common.Free, "", startTime), + newResource("fd957edc-4148-49e8-af83-53d38bcd4e60", "test-resource", common.Free, "", startTime), + &crds.DRLCObject{ + ObjectMeta: metav1.ObjectMeta{Name: "test-resource"}, + Spec: crds.DRLCSpec{ + MinCount: 10, + MaxCount: 10, + }, + }, + }, + expectedRes: &crds.ResourceObjectList{Items: []crds.ResourceObject{ + *newResource("test-resource-0", "test-resource", common.Free, "", startTime), + *newResource("test-resource-1", "test-resource", common.Free, "", startTime), + *newResource("test-resource-2", "test-resource", common.Free, "", startTime), + *newResource("fd957edc-4148-49e8-af83-53d38bcd4e54", "test-resource", common.ToBeDeleted, "", fakeNow), + *newResource("fd957edc-4148-49e8-af83-53d38bcd4e55", "test-resource", common.ToBeDeleted, "", fakeNow), + *newResource("fd957edc-4148-49e8-af83-53d38bcd4e56", "test-resource", common.ToBeDeleted, "", fakeNow), + *newResource("fd957edc-4148-49e8-af83-53d38bcd4e57", "test-resource", common.ToBeDeleted, "", fakeNow), + *newResource("fd957edc-4148-49e8-af83-53d38bcd4e58", "test-resource", common.ToBeDeleted, "", fakeNow), + *newResource("fd957edc-4148-49e8-af83-53d38bcd4e59", "test-resource", common.ToBeDeleted, "", fakeNow), + *newResource("fd957edc-4148-49e8-af83-53d38bcd4e60", "test-resource", common.ToBeDeleted, "", fakeNow), + }}, + expectedLCs: &crds.DRLCObjectList{Items: []crds.DRLCObject{{ + ObjectMeta: metav1.ObjectMeta{Name: "test-resource"}, + Spec: crds.DRLCSpec{ + MinCount: 0, + MaxCount: 0, + }, + }}}, + }, } for _, tc := range testcases { + tc := tc t.Run(tc.name, func(t *testing.T) { + t.Parallel() c := makeTestRanch(tc.currentRes) if err := c.Storage.SyncResources(tc.config); err != nil { t.Fatalf("syncResources failed: %v, type: %T", err, err) @@ -1408,7 +1495,7 @@ func TestSyncResources(t *testing.T) { tc.expectedRes.Items[idx].Status.UserData = &common.UserData{} } } - if diff := compareResourceObjectsLists(resources, tc.expectedRes); diff != nil { + if diff := compareResourceObjectsLists(resources, tc.expectedRes); diff != "" { t.Errorf("received resource differs from expected, diff: %v", diff) } lfs, err := c.Storage.GetDynamicResourceLifeCycles() @@ -1421,8 +1508,8 @@ func TestSyncResources(t *testing.T) { for idx := range tc.expectedLCs.Items { tc.expectedLCs.Items[idx].Namespace = testNS } - if diff := compareDRLCLists(lfs, tc.expectedLCs); diff != nil { - t.Errorf("received drlc do not match expected, diff: %v", deep.Equal(lfs, tc.expectedLCs)) + if diff := compareDRLCLists(lfs, tc.expectedLCs); diff != "" { + t.Errorf("received drlc do not match expected, diff: %s", diff) } }) } @@ -1704,9 +1791,11 @@ func TestUpdateAllDynamicResources(t *testing.T) { } for _, tc := range testcases { + tc := tc t.Run(tc.name, func(t *testing.T) { + t.Parallel() c := makeTestRanch(tc.currentRes) - err := c.Storage.UpdateAllDynamicResources() + err := c.Storage.UpdateAllDynamicResources(nil) if err != nil { t.Fatalf("error updating dynamic resources: %v", err) } @@ -1733,22 +1822,22 @@ func TestUpdateAllDynamicResources(t *testing.T) { } } - if diff := compareResourceObjectsLists(resources, tc.expectedRes); diff != nil { - t.Errorf("diff:\n%v", deep.Equal(resources, tc.expectedRes)) + if diff := compareResourceObjectsLists(resources, tc.expectedRes); diff != "" { + t.Errorf("diff:\n%v", diff) } lfs, err := c.Storage.GetDynamicResourceLifeCycles() if err != nil { t.Fatalf("failed to get dynamic resource life cycles: %v", err) } - if diff := compareDRLCLists(lfs, tc.expectedLCs); diff != nil { - t.Errorf("diff: %v", deep.Equal(lfs, tc.expectedLCs)) + if diff := compareDRLCLists(lfs, tc.expectedLCs); diff != "" { + t.Errorf("diff: %s", diff) } }) } } -func compareResourceObjectsLists(a, b *crds.ResourceObjectList) []string { +func compareResourceObjectsLists(a, b *crds.ResourceObjectList) string { sortResourcesLists(a, b) a.TypeMeta = metav1.TypeMeta{} a.ResourceVersion = "" @@ -1762,10 +1851,10 @@ func compareResourceObjectsLists(a, b *crds.ResourceObjectList) []string { b.Items[idx].TypeMeta = metav1.TypeMeta{} b.Items[idx].ResourceVersion = "" } - return deep.Equal(a, b) + return cmp.Diff(a, b, cmp.AllowUnexported(sync.Map{}, sync.Mutex{}, atomic.Value{})) } -func compareDRLCLists(a, b *crds.DRLCObjectList) []string { +func compareDRLCLists(a, b *crds.DRLCObjectList) string { sortDRLCList(a, b) a.TypeMeta = metav1.TypeMeta{} a.ResourceVersion = "" @@ -1779,7 +1868,7 @@ func compareDRLCLists(a, b *crds.DRLCObjectList) []string { b.Items[idx].TypeMeta = metav1.TypeMeta{} b.Items[idx].ResourceVersion = "" } - return deep.Equal(a, b) + return cmp.Diff(a, b) } func newResource(name, rtype, state, owner string, t time.Time) *crds.ResourceObject { diff --git a/ranch/storage.go b/ranch/storage.go index ae1754b9..50a45fbd 100644 --- a/ranch/storage.go +++ b/ranch/storage.go @@ -25,10 +25,12 @@ import ( "time" "github.com/sirupsen/logrus" + kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -140,7 +142,21 @@ func (s *Storage) DeleteDynamicResourceLifeCycle(name string) error { Namespace: s.namespace, }, } - return s.client.Delete(s.ctx, o) + if err := s.client.Delete(s.ctx, o); err != nil { + return err + } + if err := wait.Poll(100*time.Millisecond, 5*time.Second, func() (bool, error) { + if err := s.client.Get(s.ctx, ctrlruntimeclient.ObjectKey{Namespace: s.namespace, Name: name}, o); err != nil { + if kerrors.IsNotFound(err) { + return true, nil + } + return false, err + } + return false, nil + }); err != nil { + return fmt.Errorf("failed for deleted dynamic resource lifecycle %s/%s to vanish from cache: %w", s.namespace, name, err) + } + return nil } // UpdateDynamicResourceLifeCycle updates a dynamic resource life cycle. if it exists, errors otherwise @@ -150,6 +166,19 @@ func (s *Storage) UpdateDynamicResourceLifeCycle(resource *crds.DRLCObject) (*cr return nil, fmt.Errorf("failed to update dlrc %s: %w", resource.Name, err) } + // Make sure we have this change in our cache + expectedSpec := resource.Spec + name := ctrlruntimeclient.ObjectKey{Namespace: resource.Namespace, Name: resource.Name} + if err := wait.Poll(100*time.Millisecond, 5*time.Second, func() (bool, error) { + drlc := &crds.DRLCObject{} + if err := s.client.Get(s.ctx, name, drlc); err != nil { + return false, err + } + return reflect.DeepEqual(expectedSpec, drlc.Spec), nil + }); err != nil { + return nil, fmt.Errorf("failed waiting for object update of %s to appear in cache: %w", name, err) + } + return resource, nil } @@ -184,8 +213,9 @@ func (s *Storage) SyncResources(config *common.BoskosConfig) error { return nil } + var staticResourcesFromConfigByName map[string]crds.ResourceObject if err := retryOnConflict(retry.DefaultBackoff, func() error { - newSRByName := map[string]crds.ResourceObject{} + staticResourcesFromConfigByName = map[string]crds.ResourceObject{} existingSRByName := map[string]crds.ResourceObject{} newDRLCByType := map[string]crds.DRLCObject{} existingDRLCByType := map[string]crds.DRLCObject{} @@ -195,7 +225,7 @@ func (s *Storage) SyncResources(config *common.BoskosConfig) error { newDRLCByType[entry.Type] = *crds.FromDynamicResourceLifecycle(common.NewDynamicResourceLifeCycleFromConfig(entry)) } else { for _, res := range common.NewResourcesFromConfig(entry) { - newSRByName[res.Name] = *crds.FromResource(res) + staticResourcesFromConfigByName[res.Name] = *crds.FromResource(res) } } } @@ -229,18 +259,19 @@ func (s *Storage) SyncResources(config *common.BoskosConfig) error { } for _, res := range resources.Items { - if !lifeCycleTypes.Has(res.Spec.Type) { + if _, inStaticConfig := staticResourcesFromConfigByName[res.Name]; inStaticConfig || !lifeCycleTypes.Has(res.Spec.Type) { existingSRByName[res.Name] = res } } - if err := s.syncStaticResources(newSRByName, existingSRByName); err != nil { - return err + var errs []error + if err := s.syncStaticResources(staticResourcesFromConfigByName, existingSRByName); err != nil { + errs = append(errs, fmt.Errorf("failed to sync static resources: %w", err)) } if err := s.syncDynamicResourceLifeCycles(newDRLCByType, existingDRLCByType); err != nil { - return err + errs = append(errs, fmt.Errorf("failed to sync dynamic resources: %w", err)) } - return nil + return utilerrors.NewAggregate(errs) }(); err != nil { return err } @@ -250,7 +281,7 @@ func (s *Storage) SyncResources(config *common.BoskosConfig) error { return err } - if err := s.UpdateAllDynamicResources(); err != nil { + if err := s.UpdateAllDynamicResources(staticResourcesFromConfigByName); err != nil { logrus.WithError(err).Error("Encountered error updating dynamic resources.") return err } @@ -314,7 +345,7 @@ func (s *Storage) updateDynamicResources(lifecycle *crds.DRLCObject, resources [ toDelete = append(toDelete, res) } } - logrus.Infof("DRLC type %s: adding %+v, deleting %+v", lifecycle.Name, toAdd, toDelete) + return } @@ -323,7 +354,8 @@ func (s *Storage) updateDynamicResources(lifecycle *crds.DRLCObject, resources [ // This ensures that the MinCount and MaxCount parameters are honored, that // any expired resources are deleted, and that any Tombstoned resources are // completely removed. -func (s *Storage) UpdateAllDynamicResources() error { +// nolint:gocognit +func (s *Storage) UpdateAllDynamicResources(staticResources map[string]crds.ResourceObject) error { s.resourcesLock.Lock() defer s.resourcesLock.Unlock() @@ -347,6 +379,10 @@ func (s *Storage) UpdateAllDynamicResources() error { // Filter to only look at dynamic resources for _, res := range resources.Items { + // Do not delete objects that have a static config + if _, hasStaticConfig := staticResources[res.Name]; hasStaticConfig { + continue + } if _, ok := existingDRLCByType[res.Spec.Type]; ok { existingDRsByType[res.Spec.Type] = append(existingDRsByType[res.Spec.Type], res) } @@ -373,7 +409,7 @@ func (s *Storage) UpdateAllDynamicResources() error { } if len(dRLCToDelete) > 0 { - if err := s.persistDynamicResourceLifeCycles(nil, nil, dRLCToDelete); err != nil { + if err := s.deleteDynamicResourceLifecycles(dRLCToDelete, staticResources); err != nil { return err } } @@ -390,6 +426,7 @@ func (s *Storage) UpdateAllDynamicResources() error { func (s *Storage) syncDynamicResourceLifeCycles(newDRLCByType, existingDRLCByType map[string]crds.DRLCObject) error { var dRLCToUpdate, dRLCToAdd []crds.DRLCObject + var errs []error for _, existingDRLC := range existingDRLCByType { newDRLC, existsInNew := newDRLCByType[existingDRLC.Name] if existsInNew { @@ -399,7 +436,7 @@ func (s *Storage) syncDynamicResourceLifeCycles(newDRLCByType, existingDRLCByTyp if !reflect.DeepEqual(existingDRLC, newDRLC) { dRLCToUpdate = append(dRLCToUpdate, newDRLC) } - } else { + } else if existingDRLC.Spec.MinCount != 0 || existingDRLC.Spec.MaxCount != 0 { // Mark for deletion of all associated dynamic resources. existingDRLC.Spec.MinCount = 0 existingDRLC.Spec.MaxCount = 0 @@ -414,7 +451,8 @@ func (s *Storage) syncDynamicResourceLifeCycles(newDRLCByType, existingDRLCByTyp } } - return s.persistDynamicResourceLifeCycles(dRLCToUpdate, dRLCToAdd, nil) + errs = append(errs, s.persistDynamicResourceLifeCycles(dRLCToUpdate, dRLCToAdd)) + return utilerrors.NewAggregate(errs) } func (s *Storage) persistResources(resToAdd, resToDelete []crds.ResourceObject, dynamic bool) error { @@ -424,25 +462,26 @@ func (s *Storage) persistResources(resToAdd, resToDelete []crds.ResourceObject, if r.Status.Owner != "" { continue } + l := logrus.WithField("name", r.Name) if dynamic { // Only delete resource in tombsone state and mark the other as to deleted // This is necessary for dynamic resources that depends on other resources // as they need to be released to prevent leak. if r.Status.State == common.Tombstone { - logrus.Infof("Deleting resource %s", r.Name) + l.Info("Deleting resource") if err := s.DeleteResource(r.Name); err != nil { errs = append(errs, err) } } else if r.Status.State != common.ToBeDeleted { r.Status.State = common.ToBeDeleted - logrus.Infof("Marking resource to be deleted %s", r.Name) + l.Info("Marking resource to be deleted") if _, err := s.UpdateResource(&r); err != nil { errs = append(errs, err) } } } else { // Static resources can be deleted right away. - logrus.Infof("Deleting resource %s", r.Name) + l.Info("Deleting resource") if err := s.DeleteResource(r.Name); err != nil { errs = append(errs, err) } @@ -450,7 +489,7 @@ func (s *Storage) persistResources(resToAdd, resToDelete []crds.ResourceObject, } for _, r := range resToAdd { - logrus.Infof("Adding resource %s", r.Name) + logrus.WithField("name", r.Name).Info("Adding resource") r.Status.LastUpdate = s.now() if err := s.AddResource(&r); err != nil { errs = append(errs, err) @@ -460,18 +499,40 @@ func (s *Storage) persistResources(resToAdd, resToDelete []crds.ResourceObject, return utilerrors.NewAggregate(errs) } -func (s *Storage) persistDynamicResourceLifeCycles(dRLCToUpdate, dRLCToAdd, dRLCToDelelete []crds.DRLCObject) error { +func (s *Storage) persistDynamicResourceLifeCycles(dRLCToUpdate, dRLCToAdd []crds.DRLCObject) error { var errs []error + for idx, DRLC := range dRLCToAdd { + logrus.Infof("Adding resource type life cycle %s", DRLC.Name) + if err := s.AddDynamicResourceLifeCycle(&dRLCToAdd[idx]); err != nil { + errs = append(errs, err) + } + } + + for idx, dRLC := range dRLCToUpdate { + logrus.Infof("Updating resource type life cycle %s", dRLC.Name) + if _, err := s.UpdateDynamicResourceLifeCycle(&dRLCToUpdate[idx]); err != nil { + errs = append(errs, err) + } + } + + return utilerrors.NewAggregate(errs) +} + +func (s *Storage) deleteDynamicResourceLifecycles(dRLCToDelete []crds.DRLCObject, staticResourcesByName map[string]crds.ResourceObject) error { remainingTypes := map[string]bool{} - updatedResources, err := s.GetResources() + resources, err := s.GetResources() if err != nil { return err } - for _, res := range updatedResources.Items { + for _, res := range resources.Items { + if _, hasStaticConfig := staticResourcesByName[res.Name]; hasStaticConfig { + continue + } remainingTypes[res.Spec.Type] = true } - for _, dRLC := range dRLCToDelelete { + var errs []error + for idx, dRLC := range dRLCToDelete { // Only delete a dynamic resource if all resources are gone if !remainingTypes[dRLC.Name] { logrus.Infof("Deleting resource type life cycle %s", dRLC.Name) @@ -482,21 +543,9 @@ func (s *Storage) persistDynamicResourceLifeCycles(dRLCToUpdate, dRLCToAdd, dRLC // Mark this DRLC as pending deletion by setting min and max count to zero. dRLC.Spec.MinCount = 0 dRLC.Spec.MaxCount = 0 - dRLCToUpdate = append(dRLCToUpdate, dRLC) - } - } - - for _, DRLC := range dRLCToAdd { - logrus.Infof("Adding resource type life cycle %s", DRLC.Name) - if err := s.AddDynamicResourceLifeCycle(&DRLC); err != nil { - errs = append(errs, err) - } - } - - for _, dRLC := range dRLCToUpdate { - logrus.Infof("Updating resource type life cycle %s", dRLC.Name) - if _, err := s.UpdateDynamicResourceLifeCycle(&dRLC); err != nil { - errs = append(errs, err) + if _, err := s.UpdateDynamicResourceLifeCycle(&dRLCToDelete[idx]); err != nil { + errs = append(errs, err) + } } }