Skip to content

Commit 3ec637b

Browse files
staeblerstaebler
committed
reap namespaces created for ClusterPool after clusters deleted
When the ClusterPool creates a new cluster, it creates a namespace to house the ClusterDeployment. When the ClusterDeployment is deleted, the namespace then needs to be deleted to avoid leaking namespaces. To this end, these changes add a clusterpoolnamespace controller which will sync namespaces and delete them when appropriate. All namespaces created for ClusterPools will have a hive.openshift.io/cluster-pool-name label. The new controller will delete a namespace with that label if the namespace does not contain any ClusterDeployments. The controller will wait 5 minutes after the creation of the namespace before deleting the namespace to give time for the ClusterDeployment to be created. https://issues.redhat.com/browse/CO-1014
1 parent df5e55b commit 3ec637b

5 files changed

Lines changed: 314 additions & 2 deletions

File tree

‎cmd/manager/main.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/openshift/hive/pkg/controller/clusterdeployment"
3434
"github.com/openshift/hive/pkg/controller/clusterdeprovision"
3535
"github.com/openshift/hive/pkg/controller/clusterpool"
36+
"github.com/openshift/hive/pkg/controller/clusterpoolnamespace"
3637
"github.com/openshift/hive/pkg/controller/clusterprovision"
3738
"github.com/openshift/hive/pkg/controller/clusterrelocate"
3839
"github.com/openshift/hive/pkg/controller/clusterstate"
@@ -65,6 +66,7 @@ type controllerSetupFunc func(manager.Manager) error
6566
var controllerFuncs = map[string]controllerSetupFunc{
6667
clusterdeployment.ControllerName: clusterdeployment.Add,
6768
clusterdeprovision.ControllerName: clusterdeprovision.Add,
69+
clusterpoolnamespace.ControllerName: clusterpoolnamespace.Add,
6870
clusterprovision.ControllerName: clusterprovision.Add,
6971
clusterrelocate.ControllerName: clusterrelocate.Add,
7072
clusterstate.ControllerName: clusterstate.Add,

‎config/controllers/hive_controllers_role.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ rules:
4343
- ""
4444
resources:
4545
- pods
46-
- namespaces
4746
verbs:
4847
- get
4948
- list

‎pkg/controller/clusterpoolnamespace/clusterpoolnamespace_controller.go‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package clusterpoolnamespace
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"time"
7+
8+
log "github.com/sirupsen/logrus"
9+
corev1 "k8s.io/api/core/v1"
10+
apierrors "k8s.io/apimachinery/pkg/api/errors"
11+
"k8s.io/apimachinery/pkg/types"
12+
13+
"sigs.k8s.io/controller-runtime/pkg/client"
14+
"sigs.k8s.io/controller-runtime/pkg/controller"
15+
"sigs.k8s.io/controller-runtime/pkg/handler"
16+
"sigs.k8s.io/controller-runtime/pkg/manager"
17+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
18+
"sigs.k8s.io/controller-runtime/pkg/source"
19+
20+
hivev1 "github.com/openshift/hive/pkg/apis/hive/v1"
21+
"github.com/openshift/hive/pkg/constants"
22+
hivemetrics "github.com/openshift/hive/pkg/controller/metrics"
23+
controllerutils "github.com/openshift/hive/pkg/controller/utils"
24+
)
25+
26+
const (
27+
ControllerName = "clusterpoolnamespace"
28+
minimumLifetime = 5 * time.Minute
29+
durationBetweenDeletingClusterDeploymentChecks = 1 * time.Minute
30+
)
31+
32+
// Add creates a new ClusterDeployment Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller
33+
// and Start it when the Manager is Started.
34+
func Add(mgr manager.Manager) error {
35+
return AddToManager(mgr, NewReconciler(mgr))
36+
}
37+
38+
// NewReconciler returns a new reconcile.Reconciler
39+
func NewReconciler(mgr manager.Manager) reconcile.Reconciler {
40+
r := &ReconcileClusterPoolNamespace{
41+
Client: controllerutils.NewClientWithMetricsOrDie(mgr, ControllerName),
42+
logger: log.WithField("controller", ControllerName),
43+
}
44+
return r
45+
}
46+
47+
// AddToManager adds a new Controller to mgr with r as the reconcile.Reconciler
48+
func AddToManager(mgr manager.Manager, r reconcile.Reconciler) error {
49+
// Create a new controller
50+
c, err := controller.New(
51+
fmt.Sprintf("%s-controller", ControllerName),
52+
mgr,
53+
controller.Options{
54+
Reconciler: r,
55+
MaxConcurrentReconciles: controllerutils.GetConcurrentReconciles(),
56+
},
57+
)
58+
if err != nil {
59+
return err
60+
}
61+
62+
// Watch for changes to Namespaces
63+
if err := c.Watch(&source.Kind{Type: &corev1.Namespace{}}, &handler.EnqueueRequestForObject{}); err != nil {
64+
return err
65+
}
66+
67+
// Watch for changes to ClusterDeployment
68+
cdMapFn := func(a handler.MapObject) []reconcile.Request {
69+
cd := a.Object.(*hivev1.ClusterDeployment)
70+
return []reconcile.Request{{
71+
NamespacedName: types.NamespacedName{Name: cd.Namespace},
72+
}}
73+
}
74+
if err := c.Watch(
75+
&source.Kind{Type: &hivev1.ClusterDeployment{}},
76+
&handler.EnqueueRequestsFromMapFunc{
77+
ToRequests: handler.ToRequestsFunc(cdMapFn),
78+
},
79+
); err != nil {
80+
return err
81+
}
82+
83+
return nil
84+
}
85+
86+
var _ reconcile.Reconciler = &ReconcileClusterPoolNamespace{}
87+
88+
// ReconcileClusterPoolNamespace reconciles a Namespace object for the purpose of reaping namespaces created for
89+
// ClusterPool clusters after the clusters have been deleted.
90+
type ReconcileClusterPoolNamespace struct {
91+
client.Client
92+
logger log.FieldLogger
93+
}
94+
95+
// Reconcile deletes a Namespace if it no longer contains any ClusterDeployments.
96+
func (r *ReconcileClusterPoolNamespace) Reconcile(request reconcile.Request) (reconcile.Result, error) {
97+
start := time.Now()
98+
logger := r.logger.WithField("namespace", request.Name)
99+
100+
logger.Info("reconciling namespace")
101+
defer func() {
102+
dur := time.Since(start)
103+
hivemetrics.MetricControllerReconcileTime.WithLabelValues(ControllerName).Observe(dur.Seconds())
104+
logger.WithField("elapsed", dur).Info("reconcile complete")
105+
}()
106+
107+
// Fetch the Namespace instance
108+
namespace := &corev1.Namespace{}
109+
switch err := r.Get(context.Background(), request.NamespacedName, namespace); {
110+
case apierrors.IsNotFound(err):
111+
return reconcile.Result{}, nil
112+
case err != nil:
113+
return reconcile.Result{}, err
114+
}
115+
116+
// If the Namespace is deleted, do not reconcile.
117+
if namespace.DeletionTimestamp != nil {
118+
return reconcile.Result{}, nil
119+
}
120+
121+
// If the namespace was not created for a ClusterPool cluster, ignore it
122+
if _, ok := namespace.Labels[constants.ClusterPoolNameLabel]; !ok {
123+
return reconcile.Result{}, nil
124+
}
125+
126+
if lifetime := time.Since(namespace.CreationTimestamp.Time); lifetime < minimumLifetime {
127+
logger.WithField("lifetime", lifetime).Debug("namespace is not old enough to delete; waiting longer for ClusterDeployment to be created")
128+
return reconcile.Result{RequeueAfter: minimumLifetime - lifetime}, nil
129+
}
130+
131+
cdList := &hivev1.ClusterDeploymentList{}
132+
if err := r.List(context.Background(), cdList, client.InNamespace(namespace.Name)); err != nil {
133+
logger.WithError(err).Log(controllerutils.LogLevel(err), "could not list ClusterDeployments")
134+
return reconcile.Result{}, err
135+
}
136+
137+
if len(cdList.Items) == 0 {
138+
logger.Info("deleting namespace since it contains no ClusterDeployments")
139+
if err := r.Delete(context.Background(), namespace); err != nil {
140+
logger.WithError(err).Log(controllerutils.LogLevel(err), "error deleting namespace")
141+
return reconcile.Result{}, err
142+
}
143+
return reconcile.Result{}, nil
144+
}
145+
146+
// If all of the ClusterDeployments have been deleted, then we need to due a Requeue After since the watch will
147+
// not trigger when the ClusterDeployment is finally removed from storage.
148+
for _, cd := range cdList.Items {
149+
if cd.DeletionTimestamp == nil {
150+
logger.WithField("clusterDeployment", cd.Name).Debug("ClusterDeployment has not been deleted")
151+
return reconcile.Result{}, nil
152+
}
153+
}
154+
155+
logger.Debug("all ClusterDeployments deleted; waiting longer for ClusterDeployments to be removed from storage")
156+
return reconcile.Result{RequeueAfter: durationBetweenDeletingClusterDeploymentChecks}, nil
157+
}

‎pkg/controller/clusterpoolnamespace/clusterpoolnamespace_controller_test.go‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
package clusterpoolnamespace
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
8+
log "github.com/sirupsen/logrus"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
corev1 "k8s.io/api/core/v1"
12+
apierrors "k8s.io/apimachinery/pkg/api/errors"
13+
"k8s.io/apimachinery/pkg/runtime"
14+
"sigs.k8s.io/controller-runtime/pkg/client"
15+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
16+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
17+
18+
hivev1 "github.com/openshift/hive/pkg/apis/hive/v1"
19+
"github.com/openshift/hive/pkg/constants"
20+
testcd "github.com/openshift/hive/pkg/test/clusterdeployment"
21+
testgeneric "github.com/openshift/hive/pkg/test/generic"
22+
testnamespace "github.com/openshift/hive/pkg/test/namespace"
23+
)
24+
25+
const (
26+
namespaceName = "test-namespace"
27+
cdName = "test-cluster-deployment"
28+
crName = "test-cluster-relocator"
29+
)
30+
31+
func TestReconcileClusterPoolNamespace_Reconcile_Movement(t *testing.T) {
32+
logger := log.New()
33+
logger.SetLevel(log.DebugLevel)
34+
35+
scheme := runtime.NewScheme()
36+
corev1.AddToScheme(scheme)
37+
hivev1.AddToScheme(scheme)
38+
39+
namespaceWithoutLabelBuilder := testnamespace.FullBuilder(namespaceName, scheme)
40+
namespaceBuilder := namespaceWithoutLabelBuilder.GenericOptions(
41+
testgeneric.WithLabel(constants.ClusterPoolNameLabel, "test-cluster-pool"),
42+
)
43+
44+
validateNoRequeueAfter := func(t *testing.T, requeueAfter time.Duration, startTime, endTime time.Time) {
45+
assert.Zero(t, requeueAfter, "unexpected non-zero requeue after")
46+
}
47+
validateWaitForCDGoneRequeueAfter := func(t *testing.T, requeueAfter time.Duration, startTime, endTime time.Time) {
48+
assert.Equal(t, durationBetweenDeletingClusterDeploymentChecks, requeueAfter, "expected requeue after for checking deleting ClusterDeployments")
49+
}
50+
51+
cases := []struct {
52+
name string
53+
namespaceBuilder testnamespace.Builder
54+
extraNamespaceOption func(startTime time.Time) testnamespace.Option
55+
resources []runtime.Object
56+
expectDeleted bool
57+
validateRequeueAfter func(t *testing.T, requeueAfter time.Duration, startTime, endTime time.Time)
58+
}{
59+
{
60+
name: "no clusterdeployments",
61+
namespaceBuilder: namespaceBuilder,
62+
expectDeleted: true,
63+
validateRequeueAfter: validateNoRequeueAfter,
64+
},
65+
{
66+
name: "non-deleted clusterdeployment",
67+
namespaceBuilder: namespaceBuilder,
68+
resources: []runtime.Object{
69+
testcd.FullBuilder(namespaceName, "test-cd", scheme).Build(),
70+
},
71+
expectDeleted: false,
72+
validateRequeueAfter: validateNoRequeueAfter,
73+
},
74+
{
75+
name: "deleted clusterdeployment",
76+
namespaceBuilder: namespaceBuilder,
77+
resources: []runtime.Object{
78+
testcd.FullBuilder(namespaceName, "test-cd", scheme).
79+
GenericOptions(testgeneric.Deleted()).
80+
Build(),
81+
},
82+
expectDeleted: false,
83+
validateRequeueAfter: validateWaitForCDGoneRequeueAfter,
84+
},
85+
{
86+
name: "deleted and non-deleted clusterdeployments",
87+
namespaceBuilder: namespaceBuilder,
88+
resources: []runtime.Object{
89+
testcd.FullBuilder(namespaceName, "test-cd-1", scheme).Build(),
90+
testcd.FullBuilder(namespaceName, "test-cd-2", scheme).
91+
GenericOptions(testgeneric.Deleted()).
92+
Build(),
93+
},
94+
expectDeleted: false,
95+
validateRequeueAfter: validateNoRequeueAfter,
96+
},
97+
{
98+
name: "deleted namespace",
99+
namespaceBuilder: namespaceBuilder.GenericOptions(testgeneric.Deleted()),
100+
expectDeleted: false,
101+
validateRequeueAfter: validateNoRequeueAfter,
102+
},
103+
{
104+
name: "namespace without clusterpool label",
105+
namespaceBuilder: namespaceWithoutLabelBuilder,
106+
expectDeleted: false,
107+
validateRequeueAfter: validateNoRequeueAfter,
108+
},
109+
{
110+
name: "namespace too young",
111+
namespaceBuilder: namespaceBuilder,
112+
extraNamespaceOption: func(startTime time.Time) testnamespace.Option {
113+
return func(namespace *corev1.Namespace) {
114+
namespace.CreationTimestamp.Time = startTime.Add(-1 * time.Minute)
115+
}
116+
},
117+
expectDeleted: false,
118+
validateRequeueAfter: func(t *testing.T, requeueAfter time.Duration, startTime, endTime time.Time) {
119+
// CreationTimestamp only has granularity to the second, so add a 1-second buffer on each end of the limits.
120+
upperBound := minimumLifetime - 1*time.Minute + 1*time.Second
121+
lowerBound := minimumLifetime - 1*time.Minute - endTime.Sub(startTime) - 1*time.Second
122+
assert.LessOrEqual(t, lowerBound.Seconds(), requeueAfter.Seconds(), "requeue after too small for waiting for ClusterDeployment creation")
123+
assert.GreaterOrEqual(t, upperBound.Seconds(), requeueAfter.Seconds(), "requeue after too large for waiting for ClusterDeployment creation")
124+
},
125+
},
126+
}
127+
for _, tc := range cases {
128+
t.Run(tc.name, func(t *testing.T) {
129+
startTime := time.Now()
130+
if tc.namespaceBuilder != nil {
131+
builder := tc.namespaceBuilder
132+
if tc.extraNamespaceOption != nil {
133+
builder = builder.Options(tc.extraNamespaceOption(startTime))
134+
}
135+
tc.resources = append(tc.resources, builder.Build())
136+
}
137+
c := fake.NewFakeClientWithScheme(scheme, tc.resources...)
138+
139+
reconciler := &ReconcileClusterPoolNamespace{
140+
Client: c,
141+
logger: logger,
142+
}
143+
namespaceKey := client.ObjectKey{Name: namespaceName}
144+
result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: namespaceKey})
145+
endTime := time.Now()
146+
require.NoError(t, err, "unexpected error during reconcile")
147+
148+
namespace := &corev1.Namespace{}
149+
err = c.Get(context.Background(), namespaceKey, namespace)
150+
assert.Equal(t, tc.expectDeleted, apierrors.IsNotFound(err), "unexpected state of namespace existence")
151+
152+
tc.validateRequeueAfter(t, result.RequeueAfter, startTime, endTime)
153+
})
154+
}
155+
}

‎pkg/operator/assets/bindata.go‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)