Skip to content

Commit 8d6b448

Browse files
authored
Implement set-runonce-activedeadlineseconds policy (#124)
Sets activeDeadlineSeconds for run-once pods.
1 parent 33cbf64 commit 8d6b448

File tree

8 files changed

+280
-5
lines changed

8 files changed

+280
-5
lines changed

config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ type Config struct {
6666

6767
// LegacyNamespaceQuota is the default quota for namespaces if no ZoneUsageProfile is selected.
6868
LegacyNamespaceQuota int
69+
70+
// PodRunOnceActiveDeadlineSecondsOverrideAnnotation is the annotation used to override the activeDeadlineSeconds for RunOnce pods.
71+
PodRunOnceActiveDeadlineSecondsOverrideAnnotation string
72+
// PodRunOnceActiveDeadlineSecondsDefault is the default activeDeadlineSeconds for RunOnce pods.
73+
PodRunOnceActiveDeadlineSecondsDefault int
6974
}
7075

7176
func ConfigFromFile(path string) (c Config, warn []string, err error) {

config.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,8 @@ AllowedAnnotations: [appuio.io/default-node-selector]
4747
# AllowedLabels is a list of labels that are allowed on namespaces.
4848
# Supports '*' and '?' wildcards.
4949
AllowedLabels: [appuio.io/organization]
50+
51+
# PodRunOnceActiveDeadlineSecondsOverrideAnnotation is the annotation used to override the activeDeadlineSeconds for RunOnce pods.
52+
PodRunOnceActiveDeadlineSecondsOverrideAnnotation: appuio.io/active-deadline-seconds-override
53+
# PodRunOnceActiveDeadlineSecondsDefault is the default activeDeadlineSeconds for RunOnce pods.
54+
PodRunOnceActiveDeadlineSecondsDefault: 1800

config/webhook/manifests.yaml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,27 @@ webhooks:
4646
resources:
4747
- namespaces
4848
sideEffects: None
49+
- admissionReviewVersions:
50+
- v1
51+
clientConfig:
52+
service:
53+
name: webhook-service
54+
namespace: system
55+
path: /mutate-pod-run-once-active-deadline
56+
failurePolicy: Fail
57+
matchPolicy: Equivalent
58+
name: pod-run-once-active-deadline-mutator.appuio.io
59+
reinvocationPolicy: IfNeeded
60+
rules:
61+
- apiGroups:
62+
- ""
63+
apiVersions:
64+
- v1
65+
operations:
66+
- CREATE
67+
resources:
68+
- pods
69+
sideEffects: None
4970
- admissionReviewVersions:
5071
- v1
5172
clientConfig:

main.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ func main() {
8989
var legacyNamespaceQuotaEnabled bool
9090
flag.BoolVar(&legacyNamespaceQuotaEnabled, "legacy-namespace-quota-enabled", false, "Enable the legacy namespace quota controller. This controller is deprecated and will be removed in the future.")
9191

92+
var podRunOnceActiveDeadlineSecondsMutatorEnabled bool
93+
flag.BoolVar(&podRunOnceActiveDeadlineSecondsMutatorEnabled, "pod-run-once-active-deadline-seconds-mutator-enabled", false, "Enable the PodRunOnceActiveDeadlineSecondsMutator webhook. Adds .spec.activeDeadlineSeconds to pods with the restartPolicy set to 'OnFailure' or 'Never'.")
94+
9295
var qps, burst int
9396
flag.IntVar(&qps, "qps", 20, "QPS to use for the controller-runtime client")
9497
flag.IntVar(&burst, "burst", 100, "Burst to use for the controller-runtime client")
@@ -296,6 +299,18 @@ func main() {
296299
},
297300
})
298301

302+
mgr.GetWebhookServer().Register("/mutate-pod-run-once-active-deadline", &webhook.Admission{
303+
Handler: &webhooks.PodRunOnceActiveDeadlineSecondsMutator{
304+
Decoder: admission.NewDecoder(mgr.GetScheme()),
305+
Client: mgr.GetClient(),
306+
307+
Skipper: skipper.StaticSkipper{ShouldSkip: !podRunOnceActiveDeadlineSecondsMutatorEnabled},
308+
309+
OverrideAnnotation: conf.PodRunOnceActiveDeadlineSecondsOverrideAnnotation,
310+
DefaultActiveDeadlineSeconds: conf.PodRunOnceActiveDeadlineSecondsDefault,
311+
},
312+
})
313+
299314
if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
300315
setupLog.Error(err, "unable to setup health endpoint")
301316
os.Exit(1)

webhooks/pod_node_selector_mutator_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func Test_PodNodeSelectorMutator_Handle(t *testing.T) {
9393
DefaultNamespaceNodeSelectorAnnotation: nodeSelAnnotation,
9494
}
9595

96-
pod := newPod(tc.namespace, "test", tc.nodeSelector)
96+
pod := newPodWithNodeSelector(tc.namespace, "test", tc.nodeSelector)
9797
resp := subject.Handle(context.Background(), admissionRequestForObject(t, pod, scheme))
9898
t.Log("Response:", resp.Result.Reason, resp.Result.Message)
9999
require.ElementsMatch(t, tc.patch, resp.Patches)
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package webhooks
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"net/http"
7+
"strconv"
8+
9+
"gomodules.xyz/jsonpatch/v2"
10+
corev1 "k8s.io/api/core/v1"
11+
"sigs.k8s.io/controller-runtime/pkg/client"
12+
"sigs.k8s.io/controller-runtime/pkg/log"
13+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
14+
15+
"github.com/appuio/appuio-cloud-agent/skipper"
16+
)
17+
18+
// +kubebuilder:webhook:path=/mutate-pod-run-once-active-deadline,name=pod-run-once-active-deadline-mutator.appuio.io,admissionReviewVersions=v1,sideEffects=none,mutating=true,failurePolicy=Fail,groups="",resources=pods,verbs=create,versions=v1,matchPolicy=equivalent,reinvocationPolicy=IfNeeded
19+
//+kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
20+
21+
// PodRunOnceActiveDeadlineSecondsMutator adds .spec.activeDeadlineSeconds to pods with the restartPolicy set to "OnFailure" or "Never".
22+
type PodRunOnceActiveDeadlineSecondsMutator struct {
23+
Decoder admission.Decoder
24+
25+
// Client is used to fetch namespace metadata for the override annotation
26+
Client client.Reader
27+
28+
// DefaultNamespaceNodeSelectorAnnotation is the annotation to use for the default node selector
29+
OverrideAnnotation string
30+
31+
// DefaultActiveDeadlineSeconds is the default activeDeadlineSeconds to apply to pods
32+
DefaultActiveDeadlineSeconds int
33+
34+
Skipper skipper.Skipper
35+
}
36+
37+
// Handle handles the admission requests
38+
func (m *PodRunOnceActiveDeadlineSecondsMutator) Handle(ctx context.Context, req admission.Request) admission.Response {
39+
ctx = log.IntoContext(ctx, log.FromContext(ctx).
40+
WithName("webhook.pod-run-once-active-deadline-mutator.appuio.io").
41+
WithValues("id", req.UID, "user", req.UserInfo.Username).
42+
WithValues("operation", req.Operation).
43+
WithValues("namespace", req.Namespace, "name", req.Name,
44+
"group", req.Kind.Group, "version", req.Kind.Version, "kind", req.Kind.Kind))
45+
46+
return logAdmissionResponse(ctx, m.handle(ctx, req))
47+
}
48+
49+
func (m *PodRunOnceActiveDeadlineSecondsMutator) handle(ctx context.Context, req admission.Request) admission.Response {
50+
skip, err := m.Skipper.Skip(ctx, req)
51+
if err != nil {
52+
return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error while checking skipper: %w", err))
53+
}
54+
if skip {
55+
return admission.Allowed("skipped")
56+
}
57+
58+
var pod corev1.Pod
59+
if err := m.Decoder.Decode(req, &pod); err != nil {
60+
return admission.Errored(http.StatusUnprocessableEntity, err)
61+
}
62+
63+
if pod.Spec.RestartPolicy != corev1.RestartPolicyOnFailure && pod.Spec.RestartPolicy != corev1.RestartPolicyNever {
64+
return admission.Allowed(fmt.Sprintf("pod restart policy is %q, no activeDeadlineSeconds needed", pod.Spec.RestartPolicy))
65+
}
66+
67+
if pod.Spec.ActiveDeadlineSeconds != nil {
68+
return admission.Allowed("pod already has an activeDeadlineSeconds value")
69+
}
70+
71+
var ns corev1.Namespace
72+
if err := m.Client.Get(ctx, client.ObjectKey{Name: req.Namespace}, &ns); err != nil {
73+
return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to fetch namespace for override annotation: %w", err))
74+
}
75+
76+
ads := m.DefaultActiveDeadlineSeconds
77+
msg := fmt.Sprintf("added default activeDeadlineSeconds %d", ads)
78+
if oa := ns.Annotations[m.OverrideAnnotation]; oa != "" {
79+
parsed, err := strconv.Atoi(oa)
80+
if err != nil {
81+
return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to parse override annotation %q for namespace %q: %w", oa, req.Namespace, err))
82+
}
83+
ads = parsed
84+
msg = fmt.Sprintf("added activeDeadlineSeconds %d from override annotation %q", ads, m.OverrideAnnotation)
85+
}
86+
87+
return admission.Patched(msg, jsonpatch.Operation{
88+
Operation: "add",
89+
Path: "/spec/restartPolicy",
90+
Value: ads,
91+
})
92+
}
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
package webhooks
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
corev1 "k8s.io/api/core/v1"
9+
"k8s.io/utils/ptr"
10+
"sigs.k8s.io/controller-runtime/pkg/client"
11+
12+
"github.com/appuio/appuio-cloud-agent/skipper"
13+
)
14+
15+
func Test_PodRunOnceActiveDeadlineSecondsMutator_Handle(t *testing.T) {
16+
const overrideAnnotation = "appuio.io/active-deadline-seconds-override"
17+
const defaultActiveDeadlineSeconds = 60
18+
19+
testCases := []struct {
20+
name string
21+
22+
subject client.Object
23+
additionalObjects []client.Object
24+
25+
allowed bool
26+
expectedActiveDeadlineSeconds int
27+
}{
28+
{
29+
name: "pod with restartPolicy=Always",
30+
subject: newPodWithSpec("testns", "pod1", corev1.PodSpec{
31+
RestartPolicy: corev1.RestartPolicyAlways,
32+
}),
33+
additionalObjects: []client.Object{
34+
newNamespace("testns", nil, nil),
35+
},
36+
allowed: true,
37+
},
38+
{
39+
name: "pod with restartPolicy=OnFailure",
40+
subject: newPodWithSpec("testns", "pod1", corev1.PodSpec{
41+
RestartPolicy: corev1.RestartPolicyOnFailure,
42+
}),
43+
additionalObjects: []client.Object{
44+
newNamespace("testns", nil, nil),
45+
},
46+
allowed: true,
47+
expectedActiveDeadlineSeconds: defaultActiveDeadlineSeconds,
48+
},
49+
{
50+
name: "pod with restartPolicy=Never",
51+
subject: newPodWithSpec("testns", "pod1", corev1.PodSpec{
52+
RestartPolicy: corev1.RestartPolicyNever,
53+
}),
54+
additionalObjects: []client.Object{
55+
newNamespace("testns", nil, nil),
56+
},
57+
allowed: true,
58+
expectedActiveDeadlineSeconds: defaultActiveDeadlineSeconds,
59+
},
60+
{
61+
name: "pod in namespace with override annotation",
62+
subject: newPodWithSpec("testns", "pod1", corev1.PodSpec{
63+
RestartPolicy: corev1.RestartPolicyNever,
64+
}),
65+
additionalObjects: []client.Object{
66+
newNamespace("testns", nil, map[string]string{
67+
overrideAnnotation: "30",
68+
}),
69+
},
70+
allowed: true,
71+
expectedActiveDeadlineSeconds: 30,
72+
},
73+
{
74+
name: "pod with existing activeDeadlineSeconds",
75+
subject: newPodWithSpec("testns", "pod1", corev1.PodSpec{
76+
RestartPolicy: corev1.RestartPolicyNever,
77+
ActiveDeadlineSeconds: ptr.To(int64(77)),
78+
}),
79+
additionalObjects: []client.Object{
80+
newNamespace("testns", nil, nil),
81+
},
82+
allowed: true,
83+
},
84+
{
85+
name: "pod in namespace with invalid override annotation",
86+
subject: newPodWithSpec("testns", "pod1", corev1.PodSpec{
87+
RestartPolicy: corev1.RestartPolicyNever,
88+
}),
89+
additionalObjects: []client.Object{
90+
newNamespace("testns", nil, map[string]string{
91+
overrideAnnotation: "invalid",
92+
}),
93+
},
94+
allowed: false,
95+
},
96+
{
97+
name: "non-existing namespace",
98+
subject: newPodWithSpec("testns", "pod1", corev1.PodSpec{
99+
RestartPolicy: corev1.RestartPolicyNever,
100+
}),
101+
allowed: false,
102+
},
103+
}
104+
105+
for _, tc := range testCases {
106+
t.Run(tc.name, func(t *testing.T) {
107+
t.Parallel()
108+
109+
c, scheme, decoder := prepareClient(t, tc.additionalObjects...)
110+
111+
subject := PodRunOnceActiveDeadlineSecondsMutator{
112+
Decoder: decoder,
113+
Client: c,
114+
Skipper: skipper.StaticSkipper{},
115+
116+
OverrideAnnotation: overrideAnnotation,
117+
DefaultActiveDeadlineSeconds: defaultActiveDeadlineSeconds,
118+
}
119+
120+
resp := subject.Handle(context.Background(), admissionRequestForObject(t, tc.subject, scheme))
121+
t.Log("Response:", resp.Result.Reason, resp.Result.Message)
122+
require.Equal(t, tc.allowed, resp.Allowed)
123+
124+
if tc.expectedActiveDeadlineSeconds == 0 {
125+
require.Len(t, resp.Patches, 0)
126+
return
127+
}
128+
129+
require.Len(t, resp.Patches, 1)
130+
require.Equal(t, tc.expectedActiveDeadlineSeconds, resp.Patches[0].Value)
131+
})
132+
}
133+
}

webhooks/utils_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,13 @@ func prepareClient(t *testing.T, initObjs ...client.Object) (client.WithWatch, *
132132
return client, scheme, decoder
133133
}
134134

135-
func newPod(namespace, name string, nodeSelector map[string]string) *corev1.Pod {
135+
func newPodWithNodeSelector(namespace, name string, nodeSelector map[string]string) *corev1.Pod {
136+
return newPodWithSpec(namespace, name, corev1.PodSpec{
137+
NodeSelector: nodeSelector,
138+
})
139+
}
140+
141+
func newPodWithSpec(namespace, name string, spec corev1.PodSpec) *corev1.Pod {
136142
return &corev1.Pod{
137143
TypeMeta: metav1.TypeMeta{
138144
Kind: "Pod",
@@ -142,8 +148,6 @@ func newPod(namespace, name string, nodeSelector map[string]string) *corev1.Pod
142148
Name: name,
143149
Namespace: namespace,
144150
},
145-
Spec: corev1.PodSpec{
146-
NodeSelector: nodeSelector,
147-
},
151+
Spec: spec,
148152
}
149153
}

0 commit comments

Comments
 (0)