Refactor container deployer (run-int-tests) (#862)

* refactor pod reconciler (run-int-tests) * refactor pod reconciler (run-int-tests) * refactor pod reconciler (run-int-tests) * refactor pod reconciler (run-int-tests)
gardener · Oct 26, 2023 · 7e9d831 · 7e9d831
1 parent 57d1cca
commit 7e9d831
Show file tree

Hide file tree

Showing 21 changed files with 243 additions and 478 deletions.
diff --git a/apis/.schemes/container-v1alpha1-Configuration.json b/apis/.schemes/container-v1alpha1-Configuration.json
@@ -141,7 +141,7 @@
           "default": false
         },
         "requeueTimeSeconds": {
-          "description": "RequeueTime specifies the duration after which the object, which is not yet ready to be garbage collected, is requeued. Defaults to 60.",
+          "description": "RequeueTime specifies the duration after which the object, which is not yet ready to be garbage collected, is requeued. Defaults to 3600.",
           "type": "integer",
           "format": "int32",
           "default": 0

diff --git a/apis/deployer/container/types_config.go b/apis/deployer/container/types_config.go
@@ -106,7 +106,7 @@ type GarbageCollection struct {
 	// Defaults to 5.
 	Worker int `json:"worker"`
 	// RequeueTime specifies the duration after which the object, which is not yet ready to be garbage collected, is requeued.
-	// Defaults to 60.
+	// Defaults to 3600.
 	RequeueTimeSeconds int `json:"requeueTimeSeconds"`
 }
 

diff --git a/apis/deployer/container/v1alpha1/defaults.go b/apis/deployer/container/v1alpha1/defaults.go
@@ -30,6 +30,6 @@ func SetDefaults_GarbageCollection(obj *GarbageCollection) {
 		obj.Worker = 5
 	}
 	if obj.RequeueTimeSeconds <= 0 {
-		obj.RequeueTimeSeconds = 60
+		obj.RequeueTimeSeconds = 60 * 60
 	}
 }
diff --git a/apis/deployer/container/v1alpha1/types_config.go b/apis/deployer/container/v1alpha1/types_config.go
@@ -108,7 +108,7 @@ type GarbageCollection struct {
 	// Defaults to 5.
 	Worker int `json:"worker"`
 	// RequeueTime specifies the duration after which the object, which is not yet ready to be garbage collected, is requeued.
-	// Defaults to 60.
+	// Defaults to 3600.
 	RequeueTimeSeconds int `json:"requeueTimeSeconds"`
 }
 

diff --git a/apis/openapi/openapi_generated.go b/apis/openapi/openapi_generated.go
diff --git a/cmd/container-deployer/container-deployer-controller/app/app.go b/cmd/container-deployer/container-deployer-controller/app/app.go
@@ -36,12 +36,19 @@ func NewContainerDeployerControllerCommand(ctx context.Context) *cobra.Command {
 
 func (o *options) run(ctx context.Context) error {
 	o.DeployerOptions.Log.Info("Starting Container Deployer", lc.KeyVersion, version.Get().GitVersion)
-	if err := containerctlr.AddControllerToManager(o.DeployerOptions.Log,
+
+	gc, err := containerctlr.AddControllerToManager(o.DeployerOptions.Log,
 		o.DeployerOptions.HostMgr,
 		o.DeployerOptions.LsMgr,
 		o.Config,
-		"container"); err != nil {
-		return fmt.Errorf("unable to setup helm controller")
+		"container")
+	if err != nil {
+		return fmt.Errorf("unable to setup container controller")
+	}
+
+	if gc == nil {
+		return o.DeployerOptions.StartManagers(ctx)
+	} else {
+		return o.DeployerOptions.StartManagers(ctx, gc)
 	}
-	return o.DeployerOptions.StartManagers(ctx)
 }
diff --git a/docs/api-reference/core.md b/docs/api-reference/core.md
@@ -99,7 +99,7 @@ e.g. &ldquo;<a href="https://json-schema.org/draft/2019-09/schema&quot;">https:/
 <code>localTypes</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.JSONSchemaDefinition">
-map[string]..JSONSchemaDefinition
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.JSONSchemaDefinition
 </a>
 </em>
 </td>
@@ -365,7 +365,7 @@ Note that the type information is used to determine the secret key and the type
 <code>configurations</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>
@@ -1069,7 +1069,7 @@ InstallationImports
 <code>importDataMappings</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>
@@ -1100,7 +1100,7 @@ InstallationExports
 <code>exportDataMappings</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>
@@ -1223,7 +1223,7 @@ InstallationImports
 <code>importDataMappings</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>
@@ -1254,7 +1254,7 @@ InstallationExports
 <code>exportDataMappings</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>
@@ -2876,8 +2876,8 @@ Error
 <td>
 <code>lastErrors</code></br>
 <em>
-<a href="#landscaper.gardener.cloud/v1alpha1.*..Error">
-[]*..Error
+<a href="#landscaper.gardener.cloud/v1alpha1.*github.com/gardener/landscaper/apis/core/v1alpha1.Error">
+[]*github.com/gardener/landscaper/apis/core/v1alpha1.Error
 </a>
 </em>
 </td>
@@ -3273,7 +3273,7 @@ InstallationImports
 <code>importDataMappings</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>
@@ -4459,7 +4459,7 @@ InstallationImports
 <code>importDataMappings</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>
@@ -4490,7 +4490,7 @@ InstallationExports
 <code>exportDataMappings</code></br>
 <em>
 <a href="#landscaper.gardener.cloud/v1alpha1.AnyJSON">
-map[string]..AnyJSON
+map[string]github.com/gardener/landscaper/apis/core/v1alpha1.AnyJSON
 </a>
 </em>
 </td>

diff --git a/pkg/deployer/container/add.go b/pkg/deployer/container/add.go
@@ -7,14 +7,9 @@ package container
 import (
 	"fmt"
 
-	"github.com/go-logr/logr"
-	corev1 "k8s.io/api/core/v1"
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	containerv1alpha1 "github.com/gardener/landscaper/apis/deployer/container/v1alpha1"
 	"github.com/gardener/landscaper/controller-utils/pkg/logging"
@@ -25,7 +20,7 @@ import (
 
 // AddControllerToManager adds all necessary deployer controllers to a controller manager.
 func AddControllerToManager(logger logging.Logger, hostMgr, lsMgr manager.Manager, config containerv1alpha1.Configuration,
-	callerName string) error {
+	callerName string) (*GarbageCollector, error) {
 	log := logger.WithName("container")
 
 	lockingEnabled := config.HPAConfiguration != nil && config.HPAConfiguration.MaxReplicas > 1
@@ -38,7 +33,7 @@ func AddControllerToManager(logger logging.Logger, hostMgr, lsMgr manager.Manage
 		Scheme: hostMgr.GetScheme(),
 	})
 	if err != nil {
-		return fmt.Errorf("unable to create direct client for the host cluster: %w", err)
+		return nil, fmt.Errorf("unable to create direct client for the host cluster: %w", err)
 	}
 	containerDeployer, err := NewDeployer(
 		log,
@@ -47,21 +42,9 @@ func AddControllerToManager(logger logging.Logger, hostMgr, lsMgr manager.Manage
 		directHostClient,
 		config)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	podRec := NewPodReconciler(
-		log.WithName("podReconciler"),
-		lsMgr.GetClient(),
-		hostMgr.GetClient(),
-		lsMgr.GetEventRecorderFor("Landscaper"),
-		config,
-		containerDeployer,
-		Type,
-		lockingEnabled,
-		callerName+"pod",
-		config.TargetSelector)
-
 	options := controller.Options{
 		MaxConcurrentReconciles: config.Controller.Workers,
 	}
@@ -79,25 +62,22 @@ func AddControllerToManager(logger logging.Logger, hostMgr, lsMgr manager.Manage
 		Options:         options,
 	}, config.Controller.Workers, lockingEnabled, callerName)
 	if err != nil {
-		return err
-	}
-
-	if err := ctrl.NewControllerManagedBy(hostMgr).
-		For(&corev1.Pod{}, builder.WithPredicates(newNamespaceAndAnnotationPredicate()), builder.OnlyMetadata).
-		WithLogConstructor(func(r *reconcile.Request) logr.Logger { return log.Logr() }).
-		Complete(podRec); err != nil {
-		return err
+		return nil, err
 	}
 
 	if config.GarbageCollection.Disable {
 		log.Info("GarbageCollector disabled")
-		return nil
+		return nil, nil
 	}
-	return NewGarbageCollector(log.WithName("garbageCollector"),
+
+	keepPods := config.DebugOptions != nil && config.DebugOptions.KeepPod
+	gc := NewGarbageCollector(log.WithName("garbageCollector"),
 		lsMgr.GetClient(),
 		hostMgr.GetClient(),
 		config.Identity,
 		config.Namespace,
-		config.GarbageCollection).
-		Add(hostMgr, config.DebugOptions != nil && config.DebugOptions.KeepPod)
+		config.GarbageCollection,
+		keepPods)
+
+	return gc, nil
 }
diff --git a/pkg/deployer/container/container_suite_test.go b/pkg/deployer/container/container_suite_test.go
@@ -78,8 +78,9 @@ var _ = Describe("Template", func() {
 		})
 		Expect(err).ToNot(HaveOccurred())
 
-		Expect(containerctlr.AddControllerToManager(logging.Discard(), mgr, mgr,
-			containerv1alpha1.Configuration{}, "template-"+utils.GetNextCounter())).To(Succeed())
+		_, err = containerctlr.AddControllerToManager(logging.Discard(), mgr, mgr,
+			containerv1alpha1.Configuration{}, "template-"+utils.GetNextCounter())
+		Expect(err).To(BeNil())
 
 		state, err = testenv.InitState(ctx)
 		Expect(err).ToNot(HaveOccurred())