Skip to content

Commit 8dffcb8

Browse files
committed
Move away from Jobs to Pods
The test-operator is using Jobs to spawn test pods even though it does not use any features of this k8s object. Plus usage of the Jobs requires creation of ServiceAccount in the target namespaces. In order to be able to create a new, SA the test-oprator has to have a rights to create new roles and rolebindings which in our case makes the attack surface larger. This patch drops the usage of Jobs and moves to Pods. Depends-On: openstack-k8s-operators/ci-framework#2604
1 parent be35b07 commit 8dffcb8

File tree

14 files changed

+468
-537
lines changed

14 files changed

+468
-537
lines changed

config/rbac/role.yaml

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,18 +59,6 @@ rules:
5959
- patch
6060
- update
6161
- watch
62-
- apiGroups:
63-
- batch
64-
resources:
65-
- jobs
66-
verbs:
67-
- create
68-
- delete
69-
- get
70-
- list
71-
- patch
72-
- update
73-
- watch
7462
- apiGroups:
7563
- k8s.cni.cncf.io
7664
resources:

controllers/ansibletest_controller.go

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"errors"
2222
"fmt"
2323
"strconv"
24-
"time"
2524

2625
"reflect"
2726

@@ -30,12 +29,10 @@ import (
3029
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
3130
"github.com/openstack-k8s-operators/lib-common/modules/common/env"
3231
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
33-
"github.com/openstack-k8s-operators/lib-common/modules/common/job"
3432
common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac"
35-
"github.com/openstack-k8s-operators/test-operator/api/v1beta1"
3633
testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
34+
v1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
3735
"github.com/openstack-k8s-operators/test-operator/pkg/ansibletest"
38-
batchv1 "k8s.io/api/batch/v1"
3936
corev1 "k8s.io/api/core/v1"
4037
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
4138
ctrl "sigs.k8s.io/controller-runtime"
@@ -54,7 +51,6 @@ func (r *AnsibleTestReconciler) GetLogger(ctx context.Context) logr.Logger {
5451
// +kubebuilder:rbac:groups=test.openstack.org,resources=ansibletests,verbs=get;list;watch;create;update;patch;delete
5552
// +kubebuilder:rbac:groups=test.openstack.org,resources=ansibletests/status,verbs=get;update;patch
5653
// +kubebuilder:rbac:groups=test.openstack.org,resources=ansibletests/finalizers,verbs=update;patch
57-
// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;patch;update;delete;
5854
// +kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch
5955
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=get;list;watch;create;update;patch
6056
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=get;list;watch;create;update;patch
@@ -143,12 +139,12 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
143139
return ctrl.Result{}, err
144140

145141
case Wait:
146-
Log.Info(InfoWaitingOnJob)
142+
Log.Info(InfoWaitingOnPod)
147143
return ctrl.Result{RequeueAfter: RequeueAfterValue}, nil
148144

149145
case EndTesting:
150-
// All jobs created by the instance were completed. Release the lock
151-
// so that other instances can spawn their jobs.
146+
// All pods created by the instance were completed. Release the lock
147+
// so that other instances can spawn their pods.
152148
if lockReleased, err := r.ReleaseLock(ctx, instance); !lockReleased {
153149
Log.Info(fmt.Sprintf(InfoCanNotReleaseLock, testOperatorLockName))
154150
return ctrl.Result{RequeueAfter: RequeueAfterValue}, err
@@ -161,7 +157,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
161157
Log.Info(InfoTestingCompleted)
162158
return ctrl.Result{}, nil
163159

164-
case CreateFirstJob:
160+
case CreateFirstPod:
165161
lockAcquired, err := r.AcquireLock(ctx, instance, helper, false)
166162
if !lockAcquired {
167163
Log.Info(fmt.Sprintf(InfoCanNotAcquireLock, testOperatorLockName))
@@ -170,7 +166,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
170166

171167
Log.Info(fmt.Sprintf(InfoCreatingFirstPod, nextWorkflowStep))
172168

173-
case CreateNextJob:
169+
case CreateNextPod:
174170
// Confirm that we still hold the lock. This is useful to check if for
175171
// example somebody / something deleted the lock and it got claimed by
176172
// another instance. This is considered to be an error state.
@@ -211,9 +207,9 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
211207

212208
instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)
213209

214-
// Create a new job
210+
// Create a new pod
215211
mountCerts := r.CheckSecretExists(ctx, instance, "combined-ca-bundle")
216-
jobName := r.GetJobName(instance, nextWorkflowStep)
212+
podName := r.GetPodName(instance, nextWorkflowStep)
217213
envVars, workflowOverrideParams := r.PrepareAnsibleEnv(instance, nextWorkflowStep)
218214
logsPVCName := r.GetPVCLogsName(instance, 0)
219215
containerImage, err := r.GetContainerImage(ctx, workflowOverrideParams["ContainerImage"], instance)
@@ -249,11 +245,10 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
249245
return rbacResult, nil
250246
}
251247
// Service account, role, binding - end
252-
253-
jobDef := ansibletest.Job(
248+
podDef := ansibletest.Pod(
254249
instance,
255250
serviceLabels,
256-
jobName,
251+
podName,
257252
logsPVCName,
258253
mountCerts,
259254
envVars,
@@ -262,19 +257,12 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
262257
containerImage,
263258
privileged,
264259
)
265-
ansibleTestsJob := job.NewJob(
266-
jobDef,
267-
testv1beta1.ConfigHash,
268-
true,
269-
time.Duration(5)*time.Second,
270-
"",
271-
)
272260

273-
ctrlResult, err = ansibleTestsJob.DoJob(ctx, helper)
261+
ctrlResult, err = r.CreatePod(ctx, *helper, podDef)
274262
if err != nil {
275-
// Creation of the ansibleTests job was not successfull.
263+
// Creation of the ansibleTests pod was not successfull.
276264
// Release the lock and allow other controllers to spawn
277-
// a job.
265+
// a pod.
278266
if lockReleased, err := r.ReleaseLock(ctx, instance); !lockReleased {
279267
return ctrl.Result{}, err
280268
}
@@ -294,7 +282,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
294282
condition.DeploymentReadyRunningMessage))
295283
return ctrlResult, nil
296284
}
297-
// Create a new job - end
285+
// Create a new pod - end
298286
Log.Info("Reconciled Service successfully")
299287
return ctrl.Result{}, nil
300288
}
@@ -303,7 +291,7 @@ func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
303291
func (r *AnsibleTestReconciler) SetupWithManager(mgr ctrl.Manager) error {
304292
return ctrl.NewControllerManagedBy(mgr).
305293
For(&testv1beta1.AnsibleTest{}).
306-
Owns(&batchv1.Job{}).
294+
Owns(&corev1.Pod{}).
307295
Owns(&corev1.Secret{}).
308296
Owns(&corev1.ConfigMap{}).
309297
Complete(r)

0 commit comments

Comments
 (0)