Skip to content

Commit f180c91

Browse files
alcounitalcounit
authored
Changes for supporting selenosis:options functionality (#3)
* Add changes for alcounit/selenosis#55
1 parent a937515 commit f180c91

File tree

2 files changed

+265
-9
lines changed

2 files changed

+265
-9
lines changed

‎controllers/browser/browser_reconciler.go‎

Lines changed: 105 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package browser
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"strconv"
78
"time"
@@ -32,8 +33,19 @@ const (
3233

3334
browserContainerName = "browser"
3435
sidecarContainerName = "seleniferous"
36+
37+
selenosisOptionsAnnotationKey = "selenosis.io/options"
3538
)
3639

40+
type SelenosisOptions struct {
41+
Labels map[string]string `json:"labels,omitempty"`
42+
Containers map[string]ContainerOption `json:"containers,omitempty"`
43+
}
44+
45+
type ContainerOption struct {
46+
Env map[string]string `json:"env,omitempty"`
47+
}
48+
3749
// +kubebuilder:rbac:groups=selenosis.io,resources=browsers,verbs=get;list;watch;create;update;patch;delete
3850
// +kubebuilder:rbac:groups=selenosis.io,resources=browsers/status,verbs=get;update;patch
3951
// +kubebuilder:rbac:groups=selenosis.io,resources=browsers/finalizers,verbs=update
@@ -360,8 +372,26 @@ func (r *BrowserReconciler) handleMissingPod(ctx context.Context, browser *brows
360372
return ctrl.Result{}, nil
361373
}
362374

375+
opts, err := parseSelenosisOptions(browser.Annotations)
376+
if err != nil {
377+
log.Error(err, "invalid selenosis options JSON")
378+
if err := r.retryStatusUpdate(ctx, browser, func(b *browserv1.Browser) {
379+
b.Status.Phase = corev1.PodFailed
380+
b.Status.Reason = "InvalidSelenosisOptions"
381+
b.Status.Message = err.Error()
382+
}); err != nil {
383+
log.Error(err, "Failed to update Browser status")
384+
return ctrl.Result{}, err
385+
}
386+
387+
log.Info("Invalid selenosis options")
388+
return ctrl.Result{}, nil
389+
}
390+
391+
log.Info("parsed selenosis options", "hasOptions", opts != nil)
392+
363393
// Create pod from template
364-
if err := r.createPod(ctx, browser, browserSpec); err != nil {
394+
if err := r.createPod(ctx, browser, browserSpec, opts); err != nil {
365395
if errors.IsAlreadyExists(err) {
366396
log.Info("Browser Pod already exists, will reconcile on next iteration")
367397
return ctrl.Result{RequeueAfter: quickCheck}, nil
@@ -375,10 +405,10 @@ func (r *BrowserReconciler) handleMissingPod(ctx context.Context, browser *brows
375405
}
376406

377407
// createPod creates a Pod for Browser with optimized memory usage
378-
func (r *BrowserReconciler) createPod(ctx context.Context, browser *browserv1.Browser, browserSpec *configv1.BrowserVersionConfigSpec) error {
408+
func (r *BrowserReconciler) createPod(ctx context.Context, browser *browserv1.Browser, browserSpec *configv1.BrowserVersionConfigSpec, opts *SelenosisOptions) error {
379409
log := logger.FromContext(ctx)
380410

381-
pod := buildBrowserPod(browser, browserSpec)
411+
pod := buildBrowserPod(browser, browserSpec, opts)
382412

383413
log.Info("BrowserPodSpec configuration applied")
384414
return r.client.Create(ctx, pod)
@@ -615,7 +645,7 @@ func (r *BrowserReconciler) deleteBrowser(ctx context.Context, browser *browserv
615645
return ctrl.Result{}, nil
616646
}
617647

618-
func buildBrowserPod(browser *browserv1.Browser, cfg *configv1.BrowserVersionConfigSpec) *corev1.Pod {
648+
func buildBrowserPod(browser *browserv1.Browser, cfg *configv1.BrowserVersionConfigSpec, opts *SelenosisOptions) *corev1.Pod {
619649
pod := &corev1.Pod{
620650
ObjectMeta: metav1.ObjectMeta{
621651
Name: browser.GetName(),
@@ -778,6 +808,9 @@ func buildBrowserPod(browser *browserv1.Browser, cfg *configv1.BrowserVersionCon
778808
pod.Annotations = map[string]string{}
779809
}
780810
for k, v := range browser.Annotations {
811+
if k == selenosisOptionsAnnotationKey {
812+
continue
813+
}
781814
pod.Annotations[k] = v
782815
}
783816
}
@@ -818,6 +851,8 @@ func buildBrowserPod(browser *browserv1.Browser, cfg *configv1.BrowserVersionCon
818851
pod.Spec.Hostname = browser.GetName()
819852
pod.Spec.RestartPolicy = corev1.RestartPolicyNever
820853

854+
applySelenosisOptions(pod, opts)
855+
821856
return pod
822857
}
823858

@@ -827,3 +862,69 @@ func lenSidecars(cfg *configv1.BrowserVersionConfigSpec) int {
827862
}
828863
return 0
829864
}
865+
866+
func parseSelenosisOptions(ann map[string]string) (*SelenosisOptions, error) {
867+
if ann == nil {
868+
return nil, nil
869+
}
870+
raw := ann[selenosisOptionsAnnotationKey]
871+
if raw == "" {
872+
return nil, nil
873+
}
874+
875+
var opts SelenosisOptions
876+
if err := json.Unmarshal([]byte(raw), &opts); err != nil {
877+
return nil, fmt.Errorf("unmarshal %s: %w", selenosisOptionsAnnotationKey, err)
878+
}
879+
return &opts, nil
880+
}
881+
882+
func applySelenosisOptions(pod *corev1.Pod, opts *SelenosisOptions) {
883+
if pod == nil || opts == nil {
884+
return
885+
}
886+
887+
if len(opts.Containers) > 0 {
888+
for i := range pod.Spec.Containers {
889+
name := pod.Spec.Containers[i].Name
890+
option, ok := opts.Containers[name]
891+
if !ok || len(option.Env) == 0 {
892+
continue
893+
}
894+
pod.Spec.Containers[i].Env = mergeEnvVars(pod.Spec.Containers[i].Env, option.Env)
895+
}
896+
}
897+
898+
if opts.Labels != nil {
899+
if pod.Labels == nil {
900+
pod.Labels = map[string]string{}
901+
}
902+
for k, v := range opts.Labels {
903+
pod.Labels[k] = v
904+
}
905+
}
906+
}
907+
908+
func mergeEnvVars(base []corev1.EnvVar, override map[string]string) []corev1.EnvVar {
909+
if len(override) == 0 {
910+
return base
911+
}
912+
913+
idx := make(map[string]int, len(base))
914+
out := append([]corev1.EnvVar(nil), base...)
915+
for i := range out {
916+
idx[out[i].Name] = i
917+
}
918+
919+
for k, v := range override {
920+
ev := corev1.EnvVar{Name: k, Value: v}
921+
if pos, ok := idx[k]; ok {
922+
out[pos] = ev
923+
} else {
924+
idx[k] = len(out)
925+
out = append(out, ev)
926+
}
927+
}
928+
929+
return out
930+
}

‎controllers/browser/browser_reconciler_test.go‎

Lines changed: 160 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"reflect"
7+
"strings"
78
"testing"
89
"time"
910
"unsafe"
@@ -55,6 +56,15 @@ func setStoreConfig(t *testing.T, cfgStore *store.BrowserConfigStore, key string
5556
m[key] = spec
5657
}
5758

59+
func envValue(env []corev1.EnvVar, key string) (string, bool) {
60+
for _, item := range env {
61+
if item.Name == key {
62+
return item.Value, true
63+
}
64+
}
65+
return "", false
66+
}
67+
5868
func TestContainerStateEqual(t *testing.T) {
5969
now := metav1.NewTime(time.Now().UTC())
6070
a := corev1.ContainerState{Running: &corev1.ContainerStateRunning{StartedAt: now}}
@@ -149,7 +159,7 @@ func TestBuildBrowserPod(t *testing.T) {
149159
},
150160
}
151161

152-
pod := buildBrowserPod(brw, cfg)
162+
pod := buildBrowserPod(brw, cfg, nil)
153163
if pod.Name != "b1" || pod.Namespace != "ns" {
154164
t.Fatalf("unexpected pod identity")
155165
}
@@ -167,6 +177,96 @@ func TestBuildBrowserPod(t *testing.T) {
167177
}
168178
}
169179

180+
func TestParseSelenosisOptionsInvalidJSON(t *testing.T) {
181+
ann := map[string]string{
182+
selenosisOptionsAnnotationKey: "{nope",
183+
}
184+
_, err := parseSelenosisOptions(ann)
185+
if err == nil {
186+
t.Fatalf("expected error")
187+
}
188+
if !strings.Contains(err.Error(), selenosisOptionsAnnotationKey) {
189+
t.Fatalf("expected error to mention annotation key, got %v", err)
190+
}
191+
}
192+
193+
func TestParseSelenosisOptionsEmpty(t *testing.T) {
194+
opts, err := parseSelenosisOptions(nil)
195+
if err != nil {
196+
t.Fatalf("expected no error, got %v", err)
197+
}
198+
if opts != nil {
199+
t.Fatalf("expected nil options for nil annotations")
200+
}
201+
202+
opts, err = parseSelenosisOptions(map[string]string{selenosisOptionsAnnotationKey: ""})
203+
if err != nil {
204+
t.Fatalf("expected no error, got %v", err)
205+
}
206+
if opts != nil {
207+
t.Fatalf("expected nil options for empty annotation")
208+
}
209+
}
210+
211+
func TestParseSelenosisOptionsValidJSON(t *testing.T) {
212+
ann := map[string]string{
213+
selenosisOptionsAnnotationKey: `{"labels":{"a":"b"},"containers":{"browser":{"env":{"X":"1"}}}}`,
214+
}
215+
opts, err := parseSelenosisOptions(ann)
216+
if err != nil {
217+
t.Fatalf("expected no error, got %v", err)
218+
}
219+
if opts == nil || opts.Labels["a"] != "b" {
220+
t.Fatalf("expected labels to be parsed")
221+
}
222+
if opts.Containers["browser"].Env["X"] != "1" {
223+
t.Fatalf("expected container env to be parsed")
224+
}
225+
}
226+
227+
func TestApplySelenosisOptionsMergesEnvAndLabels(t *testing.T) {
228+
pod := &corev1.Pod{
229+
ObjectMeta: metav1.ObjectMeta{
230+
Labels: map[string]string{"existing": "1"},
231+
},
232+
Spec: corev1.PodSpec{
233+
Containers: []corev1.Container{
234+
{
235+
Name: "browser",
236+
Env: []corev1.EnvVar{
237+
{Name: "A", Value: "1"},
238+
{Name: "B", Value: "2"},
239+
},
240+
},
241+
{Name: "sidecar"},
242+
},
243+
},
244+
}
245+
opts := &SelenosisOptions{
246+
Labels: map[string]string{"from": "options"},
247+
Containers: map[string]ContainerOption{
248+
"browser": {Env: map[string]string{"B": "override", "C": "new"}},
249+
},
250+
}
251+
252+
applySelenosisOptions(pod, opts)
253+
254+
if pod.Labels["existing"] != "1" || pod.Labels["from"] != "options" {
255+
t.Fatalf("expected labels to be merged, got %+v", pod.Labels)
256+
}
257+
258+
env := pod.Spec.Containers[0].Env
259+
if val, ok := envValue(env, "A"); !ok || val != "1" {
260+
t.Fatalf("expected env A=1")
261+
}
262+
if val, ok := envValue(env, "B"); !ok || val != "override" {
263+
t.Fatalf("expected env B override")
264+
}
265+
if val, ok := envValue(env, "C"); !ok || val != "new" {
266+
t.Fatalf("expected env C new")
267+
}
268+
}
269+
170270
func TestHandleMissingPodConfigNotFound(t *testing.T) {
171271
scheme := newBrowserScheme(t)
172272
cfgStore := store.NewBrowserConfigStore()
@@ -264,6 +364,61 @@ func TestHandleMissingPodCreatesPod(t *testing.T) {
264364
}
265365
}
266366

367+
func TestHandleMissingPodInvalidSelenosisOptions(t *testing.T) {
368+
scheme := newBrowserScheme(t)
369+
cfgStore := store.NewBrowserConfigStore()
370+
spec := &configv1.BrowserVersionConfigSpec{Image: "img"}
371+
setStoreConfig(t, cfgStore, "ns/chrome:120", spec)
372+
373+
cl := newBrowserClient(scheme)
374+
r := NewBrowserReconciler(cl, cfgStore, scheme)
375+
376+
brw := &browserv1.Browser{
377+
ObjectMeta: metav1.ObjectMeta{
378+
Name: "b1",
379+
Namespace: "ns",
380+
Annotations: map[string]string{
381+
selenosisOptionsAnnotationKey: "{bad-json",
382+
},
383+
},
384+
Spec: browserv1.BrowserSpec{
385+
BrowserName: "chrome",
386+
BrowserVersion: "120",
387+
},
388+
}
389+
if err := cl.Create(context.Background(), brw); err != nil {
390+
t.Fatalf("create browser: %v", err)
391+
}
392+
393+
res, err := r.handleMissingPod(context.Background(), brw)
394+
if err != nil {
395+
t.Fatalf("expected no error, got %v", err)
396+
}
397+
if res.RequeueAfter != 0 {
398+
t.Fatalf("expected no requeue, got %v", res.RequeueAfter)
399+
}
400+
401+
updated := &browserv1.Browser{}
402+
if err := cl.Get(context.Background(), client.ObjectKey{Name: "b1", Namespace: "ns"}, updated); err != nil {
403+
t.Fatalf("get browser: %v", err)
404+
}
405+
if updated.Status.Phase != corev1.PodFailed {
406+
t.Fatalf("expected failed status, got %s", updated.Status.Phase)
407+
}
408+
if updated.Status.Reason != "InvalidSelenosisOptions" {
409+
t.Fatalf("expected reason InvalidSelenosisOptions, got %s", updated.Status.Reason)
410+
}
411+
412+
pod := &corev1.Pod{}
413+
err = cl.Get(context.Background(), client.ObjectKey{Name: "b1", Namespace: "ns"}, pod)
414+
if err == nil {
415+
t.Fatalf("expected no pod to be created")
416+
}
417+
if !apierrors.IsNotFound(err) {
418+
t.Fatalf("expected not found error, got %v", err)
419+
}
420+
}
421+
267422
func TestUpdateBrowserStatusCriticalContainer(t *testing.T) {
268423
scheme := newBrowserScheme(t)
269424
cl := newBrowserClient(scheme)
@@ -852,7 +1007,7 @@ func TestBuildBrowserPodWithInitContainersAndVolumes(t *testing.T) {
8521007
},
8531008
}
8541009

855-
pod := buildBrowserPod(brw, cfg)
1010+
pod := buildBrowserPod(brw, cfg, nil)
8561011
if len(pod.Spec.InitContainers) != 1 {
8571012
t.Fatalf("expected init container")
8581013
}
@@ -893,7 +1048,7 @@ func TestBuildBrowserPodInitContainerFields(t *testing.T) {
8931048
},
8941049
}
8951050

896-
pod := buildBrowserPod(brw, cfg)
1051+
pod := buildBrowserPod(brw, cfg, nil)
8971052
if len(pod.Spec.InitContainers) != 1 {
8981053
t.Fatalf("expected init container")
8991054
}
@@ -962,7 +1117,7 @@ func TestBuildBrowserPodAllFields(t *testing.T) {
9621117
},
9631118
}
9641119

965-
pod := buildBrowserPod(brw, cfg)
1120+
pod := buildBrowserPod(brw, cfg, nil)
9661121
if pod.Spec.NodeSelector["k"] != "v" {
9671122
t.Fatalf("expected node selector")
9681123
}
@@ -995,7 +1150,7 @@ func TestBuildBrowserPodBrowserLabelsOnly(t *testing.T) {
9951150
},
9961151
}
9971152

998-
pod := buildBrowserPod(brw, cfg)
1153+
pod := buildBrowserPod(brw, cfg, nil)
9991154
if pod.Labels["only"] != "browser" {
10001155
t.Fatalf("expected browser labels to be applied")
10011156
}

0 commit comments

Comments
 (0)