Skip to content

Commit

Permalink
Only apply imagePullPolicy to plugins if instructed (#1705)
Browse files Browse the repository at this point in the history
Adds a new option which allows the user to specify whether or
not to apply the imagePullPolicy to all plugins and override
their stated values.

This is important since even the default value was overwriting
explicit values in the plugins leading to confusing behavior.

Techinically a breaking change for someone who was running
sonobuoy CLI with the --image-pull-policy flag to overwrite explicit
values. They will now need to add the --force-image-pull-policy flag
to obtain the same behavior. This makes all modifications to plugins
explicit via some option rather than this one being implicit.

Fixes #1652

Signed-off-by: John Schnake <[email protected]>
  • Loading branch information
johnSchnake authored May 12, 2022
1 parent b293bf0 commit 692522d
Show file tree
Hide file tree
Showing 42 changed files with 544 additions and 56 deletions.
14 changes: 12 additions & 2 deletions cmd/sonobuoy/app/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const (
namespaceFlag = "namespace"
sonobuoyImageFlag = "sonobuoy-image"
imagePullPolicyFlag = "image-pull-policy"
forceImagePullPolicyFlag = "force-image-pull-policy"
pluginFlag = "plugin"
timeoutFlag = "timeout"
waitOutputFlag = "wait-output"
Expand Down Expand Up @@ -339,11 +340,20 @@ func AddWaitOutputFlag(mode *WaitOutputMode, flags *pflag.FlagSet, defaultMode W
"Specify the type of output Sonobuoy should produce when --wait is used. Valid modes are silent, spinner, or progress")
}

// AddImagePullPolicyFlag adds a boolean flag for deleting everything (including E2E tests).
// AddImagePullPolicyFlag adds a string flag for the image pull policy for the aggregator/worker. If ForceImagePullPolicy
// is set, it will also apply to the plugins.
func AddImagePullPolicyFlag(policy *string, flags *pflag.FlagSet) {
flags.StringVar(
policy, imagePullPolicyFlag, config.DefaultSonobuoyPullPolicy,
fmt.Sprintf("Set the ImagePullPolicy for the Sonobuoy image and all plugins. Valid options are %s.", strings.Join(ValidPullPolicies(), ", ")),
fmt.Sprintf("Set the ImagePullPolicy for the Sonobuoy image and all plugins (if --%v is set). Valid options are %s.", forceImagePullPolicyFlag, strings.Join(ValidPullPolicies(), ", ")),
)
}

// AddForceImagePullPolicyFlag adds a boolean flag for applying the ImagePullPolicy to all plugins even if set in their
// plugin specification.
func AddForceImagePullPolicyFlag(apply *bool, flags *pflag.FlagSet) {
flags.BoolVar(
apply, forceImagePullPolicyFlag, false, "Force plugins' imagePullPolicy to match the value for the Sonobuoy pod",
)
}

Expand Down
1 change: 1 addition & 0 deletions cmd/sonobuoy/app/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func GenFlagSet(cfg *genFlags, rbac RBACMode) *pflag.FlagSet {
AddKubeconfigFlag(&cfg.kubecfg, genset)
AddRBACModeFlags(&cfg.rbacMode, genset, rbac)
AddImagePullPolicyFlag(&cfg.sonobuoyConfig.ImagePullPolicy, genset)
AddForceImagePullPolicyFlag(&cfg.sonobuoyConfig.ForceImagePullPolicy, genset)
AddTimeoutFlag(&cfg.sonobuoyConfig.Aggregation.TimeoutSeconds, genset)
AddShowDefaultPodSpecFlag(&cfg.showDefaultPodSpec, genset)
AddAggregatorPermissionsFlag(&cfg.sonobuoyConfig.AggregatorPermissions, genset)
Expand Down
7 changes: 4 additions & 3 deletions pkg/client/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,11 @@ func (*SonobuoyClient) GenerateManifestAndPlugins(cfg *GenConfig) ([]byte, []*ma
return strings.ToLower(plugins[i].SonobuoyConfig.PluginName) < strings.ToLower(plugins[j].SonobuoyConfig.PluginName)
})

// Apply our universal transforms; only applies to ImagePullPolicy. Overrides all
// plugin values.
// Apply our universal transforms; only override defaultImagePullPolicy if desired.
for _, p := range plugins {
p.Spec.ImagePullPolicy = corev1.PullPolicy(cfg.Config.ImagePullPolicy)
if cfg.Config.ForceImagePullPolicy {
p.Spec.ImagePullPolicy = corev1.PullPolicy(cfg.Config.ImagePullPolicy)
}
}

// Apply transforms. Ensure this is before handling configmaps and applying the k8s_version.
Expand Down
11 changes: 7 additions & 4 deletions pkg/client/gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,12 @@ func TestGenerateManifestGolden(t *testing.T) {
},
goldenFile: filepath.Join("testdata", "plugin-configmaps.golden"),
}, {
name: "ImagePullPolicy applied to all plugins",
name: "ImagePullPolicy applied to all plugins if forced",
inputcm: &client.GenConfig{
Config: staticConfig(),
Config: fromConfig(func(c *config.Config) *config.Config {
c.ForceImagePullPolicy = true
return c
}),
KubeVersion: "v99+static.testing",
StaticPlugins: []*manifest.Manifest{
{
Expand All @@ -446,7 +449,7 @@ func TestGenerateManifestGolden(t *testing.T) {
},
goldenFile: filepath.Join("testdata", "imagePullPolicy-all-plugins.golden"),
}, {
name: "ImagePullPolicy applied to all plugins",
name: "ImagePullPolicy not applied to all plugins by default",
inputcm: &client.GenConfig{
Config: staticConfig(),
KubeVersion: "v99+static.testing",
Expand All @@ -460,7 +463,7 @@ func TestGenerateManifestGolden(t *testing.T) {
},
},
},
goldenFile: filepath.Join("testdata", "imagePullPolicy-all-plugins.golden"),
goldenFile: filepath.Join("testdata", "imagePullPolicy-not-all-plugins.golden"),
}, {
name: "AggregatorPermissions for non-cluster admin",
inputcm: &client.GenConfig{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down Expand Up @@ -112,7 +111,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: sonobuoy/systemd-logs:v0.4
imagePullPolicy: IfNotPresent
name: systemd-logs
securityContext:
privileged: true
Expand Down
2 changes: 0 additions & 2 deletions pkg/client/testdata/default-plugins-via-nil-selection.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down Expand Up @@ -117,7 +116,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: sonobuoy/systemd-logs:v0.4
imagePullPolicy: IfNotPresent
name: systemd-logs
securityContext:
privileged: true
Expand Down
2 changes: 0 additions & 2 deletions pkg/client/testdata/default-plugins-via-selection.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down Expand Up @@ -117,7 +116,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: sonobuoy/systemd-logs:v0.4
imagePullPolicy: IfNotPresent
name: systemd-logs
securityContext:
privileged: true
Expand Down
2 changes: 0 additions & 2 deletions pkg/client/testdata/default-pod-spec.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down Expand Up @@ -117,7 +116,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: sonobuoy/systemd-logs:v0.4
imagePullPolicy: IfNotPresent
name: systemd-logs
securityContext:
privileged: true
Expand Down
2 changes: 0 additions & 2 deletions pkg/client/testdata/default.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down Expand Up @@ -117,7 +116,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: sonobuoy/systemd-logs:v0.4
imagePullPolicy: IfNotPresent
name: systemd-logs
securityContext:
privileged: true
Expand Down
2 changes: 0 additions & 2 deletions pkg/client/testdata/e2e-default.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down Expand Up @@ -117,7 +116,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: sonobuoy/systemd-logs:v0.4
imagePullPolicy: IfNotPresent
name: systemd-logs
securityContext:
privileged: true
Expand Down
1 change: 0 additions & 1 deletion pkg/client/testdata/e2e-progress-custom-port.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down
1 change: 0 additions & 1 deletion pkg/client/testdata/e2e-progress-vs-user-defined.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down
1 change: 0 additions & 1 deletion pkg/client/testdata/e2e-progress.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down
1 change: 0 additions & 1 deletion pkg/client/testdata/envoverrides.golden
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down
1 change: 0 additions & 1 deletion pkg/client/testdata/goRunnerRemoved.golden
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/testdata/imagePullPolicy-all-plugins.golden
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ metadata:
---
apiVersion: v1
data:
config.json: '{"Description":"DEFAULT","UUID":"","Version":"static-version-for-testing","ResultsDir":"/tmp/sonobuoy/results","Resources":null,"Filters":{"Namespaces":".*","LabelSelector":""},"Limits":{"PodLogs":{"Namespaces":"kube-system","SonobuoyNamespace":true,"FieldSelectors":[],"LabelSelector":"","Previous":false,"SinceSeconds":null,"SinceTime":null,"Timestamps":false,"TailLines":null,"LimitBytes":null}},"QPS":30,"Burst":50,"Server":{"bindaddress":"0.0.0.0","bindport":8080,"advertiseaddress":"","timeoutseconds":21600},"Plugins":null,"PluginSearchPath":["./plugins.d","/etc/sonobuoy/plugins.d","~/sonobuoy/plugins.d"],"Namespace":"sonobuoy","WorkerImage":"sonobuoy/sonobuoy:static-version-for-testing","ImagePullPolicy":"IfNotPresent","ImagePullSecrets":"","AggregatorPermissions":"clusterAdmin","ProgressUpdatesPort":"8099","SecurityContextMode":"nonroot"}'
config.json: '{"Description":"DEFAULT","UUID":"","Version":"static-version-for-testing","ResultsDir":"/tmp/sonobuoy/results","Resources":null,"Filters":{"Namespaces":".*","LabelSelector":""},"Limits":{"PodLogs":{"Namespaces":"kube-system","SonobuoyNamespace":true,"FieldSelectors":[],"LabelSelector":"","Previous":false,"SinceSeconds":null,"SinceTime":null,"Timestamps":false,"TailLines":null,"LimitBytes":null}},"QPS":30,"Burst":50,"Server":{"bindaddress":"0.0.0.0","bindport":8080,"advertiseaddress":"","timeoutseconds":21600},"Plugins":null,"PluginSearchPath":["./plugins.d","/etc/sonobuoy/plugins.d","~/sonobuoy/plugins.d"],"Namespace":"sonobuoy","WorkerImage":"sonobuoy/sonobuoy:static-version-for-testing","ImagePullPolicy":"IfNotPresent","ForceImagePullPolicy":true,"ImagePullSecrets":"","AggregatorPermissions":"clusterAdmin","ProgressUpdatesPort":"8099","SecurityContextMode":"nonroot"}'
kind: ConfigMap
metadata:
labels:
Expand Down
147 changes: 147 additions & 0 deletions pkg/client/testdata/imagePullPolicy-not-all-plugins.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
apiVersion: v1
kind: Namespace
metadata:
name: sonobuoy
---
apiVersion: v1
kind: ServiceAccount
metadata:
labels:
component: sonobuoy
name: sonobuoy-serviceaccount
namespace: sonobuoy
---
apiVersion: v1
data:
config.json: '{"Description":"DEFAULT","UUID":"","Version":"static-version-for-testing","ResultsDir":"/tmp/sonobuoy/results","Resources":null,"Filters":{"Namespaces":".*","LabelSelector":""},"Limits":{"PodLogs":{"Namespaces":"kube-system","SonobuoyNamespace":true,"FieldSelectors":[],"LabelSelector":"","Previous":false,"SinceSeconds":null,"SinceTime":null,"Timestamps":false,"TailLines":null,"LimitBytes":null}},"QPS":30,"Burst":50,"Server":{"bindaddress":"0.0.0.0","bindport":8080,"advertiseaddress":"","timeoutseconds":21600},"Plugins":null,"PluginSearchPath":["./plugins.d","/etc/sonobuoy/plugins.d","~/sonobuoy/plugins.d"],"Namespace":"sonobuoy","WorkerImage":"sonobuoy/sonobuoy:static-version-for-testing","ImagePullPolicy":"IfNotPresent","ImagePullSecrets":"","AggregatorPermissions":"clusterAdmin","ProgressUpdatesPort":"8099","SecurityContextMode":"nonroot"}'
kind: ConfigMap
metadata:
labels:
component: sonobuoy
name: sonobuoy-config-cm
namespace: sonobuoy
---
apiVersion: v1
data:
plugin-0.yaml: |-
sonobuoy-config:
driver: ""
plugin-name: myplugin1
spec:
env:
- name: RESULTS_DIR
value: /tmp/sonobuoy/results
- name: SONOBUOY
value: "true"
- name: SONOBUOY_CONFIG_DIR
value: /tmp/sonobuoy/config
- name: SONOBUOY_K8S_VERSION
value: v99+static.testing
- name: SONOBUOY_PROGRESS_PORT
value: "8099"
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
imagePullPolicy: Never
name: ""
volumeMounts:
- mountPath: /tmp/sonobuoy/results
name: results
plugin-1.yaml: |-
sonobuoy-config:
driver: ""
plugin-name: myplugin2
spec:
env:
- name: RESULTS_DIR
value: /tmp/sonobuoy/results
- name: SONOBUOY
value: "true"
- name: SONOBUOY_CONFIG_DIR
value: /tmp/sonobuoy/config
- name: SONOBUOY_K8S_VERSION
value: v99+static.testing
- name: SONOBUOY_PROGRESS_PORT
value: "8099"
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
imagePullPolicy: Always
name: ""
volumeMounts:
- mountPath: /tmp/sonobuoy/results
name: results
kind: ConfigMap
metadata:
labels:
component: sonobuoy
name: sonobuoy-plugins-cm
namespace: sonobuoy
---
apiVersion: v1
kind: Pod
metadata:
labels:
component: sonobuoy
sonobuoy-component: aggregator
tier: analysis
name: sonobuoy
namespace: sonobuoy
spec:
containers:
- args:
- aggregator
- --no-exit
- --level=info
- -v=4
- --alsologtostderr
command:
- /sonobuoy
env:
- name: SONOBUOY_ADVERTISE_IP
valueFrom:
fieldRef:
fieldPath: status.podIP
image: sonobuoy/sonobuoy:static-version-for-testing
name: kube-sonobuoy
volumeMounts:
- mountPath: /etc/sonobuoy
name: sonobuoy-config-volume
- mountPath: /plugins.d
name: sonobuoy-plugins-volume
- mountPath: /tmp/sonobuoy
name: output-volume
restartPolicy: Never
securityContext:
fsGroup: 2000
runAsGroup: 3000
runAsUser: 1000
serviceAccountName: sonobuoy-serviceaccount
tolerations:
- key: kubernetes.io/e2e-evict-taint-key
operator: Exists
volumes:
- configMap:
name: sonobuoy-config-cm
name: sonobuoy-config-volume
- configMap:
name: sonobuoy-plugins-cm
name: sonobuoy-plugins-volume
- emptyDir: {}
name: output-volume
---
apiVersion: v1
kind: Service
metadata:
labels:
component: sonobuoy
sonobuoy-component: aggregator
name: sonobuoy-aggregator
namespace: sonobuoy
spec:
ports:
- port: 8080
protocol: TCP
targetPort: 8080
selector:
sonobuoy-component: aggregator
type: ClusterIP
---
1 change: 0 additions & 1 deletion pkg/client/testdata/imagePullSecrets.golden
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ data:
- name: SONOBUOY_RESULTS_DIR
value: /tmp/sonobuoy/results
image: k8s.gcr.io/conformance:v99+static.testing
imagePullPolicy: IfNotPresent
name: e2e
volumeMounts:
- mountPath: /tmp/sonobuoy/results
Expand Down
Loading

0 comments on commit 692522d

Please sign in to comment.