Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/v1/installation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ type InstallationSpec struct {
// +optional
ImagePullSecrets []v1.LocalObjectReference `json:"imagePullSecrets,omitempty"`

// ImagePullPolicy is the pull policy applied to containers in pods rendered by the operator
// that do not explicitly set their own pull policy. If unset, defaults to IfNotPresent.
// This is useful in air-gapped environments where images are pre-loaded onto nodes and
// must not be re-pulled from a remote registry.
// +optional
// +kubebuilder:validation:Enum=Always;IfNotPresent;Never
ImagePullPolicy *v1.PullPolicy `json:"imagePullPolicy,omitempty"`

// KubernetesProvider specifies a particular provider of the Kubernetes platform and enables provider-specific configuration.
// If the specified value is empty, the Operator will attempt to automatically determine the current provider.
// If the specified value is not empty, the Operator will still attempt auto-detection, but
Expand Down
5 changes: 5 additions & 0 deletions api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 39 additions & 18 deletions pkg/controller/utils/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

v3 "github.com/tigera/api/pkg/apis/projectcalico/v3"
operatorv1 "github.com/tigera/operator/api/v1"
"github.com/tigera/operator/pkg/apigroup"
"github.com/tigera/operator/pkg/common"
"github.com/tigera/operator/pkg/controller/status"
Expand Down Expand Up @@ -230,8 +231,21 @@ func (c *componentHandler) createOrUpdateObject(ctx context.Context, obj client.
// system as specified by the osType.
ensureOSSchedulingRestrictions(obj, osType)

// Make sure any objects with images also have an image pull policy.
modifyPodSpec(obj, setImagePullPolicy)
// Look up the InstallationSpec once and reuse it for the passes that need it
// (image pull policy and TLS ciphers), so we don't pay for the same Get twice.
var installationSpec *operatorv1.InstallationSpec
if _, spec, err := GetInstallationSpec(ctx, c.client); err == nil {
installationSpec = spec
}

// Set image pull policy based on user input, if specified.
var configuredPolicy *v1.PullPolicy
if installationSpec != nil {
configuredPolicy = installationSpec.ImagePullPolicy
}
modifyPodSpec(obj, func(podSpec *v1.PodSpec) {
setImagePullPolicy(podSpec, configuredPolicy)
})
// Order volumes and volume mounts
modifyPodSpec(obj, orderVolumes)
modifyPodSpec(obj, orderVolumeMounts)
Expand All @@ -242,7 +256,7 @@ func (c *componentHandler) createOrUpdateObject(ctx context.Context, obj client.
// Make sure we have our standard selector and pod labels
setStandardSelectorAndLabels(obj, c.cr)

if err := ensureTLSCiphers(ctx, obj, c.client); err != nil {
if err := ensureTLSCiphers(obj, installationSpec); err != nil {
return fmt.Errorf("failed to set TLS Ciphers: %w", err)
}

Expand Down Expand Up @@ -796,17 +810,33 @@ func modifyPodSpec(obj client.Object, f func(*v1.PodSpec)) {
}
}

// setImagePullPolicy ensures that an image pull policy is set if not set already.
func setImagePullPolicy(podSpec *v1.PodSpec) {
for i := range podSpec.Containers {
if len(podSpec.Containers[i].ImagePullPolicy) == 0 {
podSpec.Containers[i].ImagePullPolicy = v1.PullIfNotPresent
// setImagePullPolicy applies an image pull policy to all containers and init containers in
// the given pod spec. If configuredPolicy is non-nil it is applied to every container,
// overriding any policy the renderer set — this is what lets a user force IfNotPresent or
// Never for air-gapped clusters. If configuredPolicy is nil, containers that do not already
// specify a policy fall back to IfNotPresent.
func setImagePullPolicy(podSpec *v1.PodSpec, configuredPolicy *v1.PullPolicy) {
apply := func(c *v1.Container) {
switch {
case configuredPolicy != nil:
c.ImagePullPolicy = *configuredPolicy
case c.ImagePullPolicy == "":
c.ImagePullPolicy = v1.PullIfNotPresent
}
}
for i := range podSpec.Containers {
apply(&podSpec.Containers[i])
}
for i := range podSpec.InitContainers {
apply(&podSpec.InitContainers[i])
}
}

// ensureTLSCiphers sets the TLSCipherSuites configuration as a Env Var to the Deployments and DaemonSets.
func ensureTLSCiphers(ctx context.Context, obj client.Object, c client.Client) error {
func ensureTLSCiphers(obj client.Object, installationSpec *operatorv1.InstallationSpec) error {
if installationSpec == nil {
return nil
}
var containers []v1.Container
switch obj := obj.(type) {
case *apps.Deployment:
Expand All @@ -817,15 +847,6 @@ func ensureTLSCiphers(ctx context.Context, obj client.Object, c client.Client) e
return nil
}

_, installationSpec, err := GetInstallationSpec(ctx, c)
if err != nil {
if errors.IsNotFound(err) {
return nil
} else {
return err
}
}

for i := range containers {
exists := false
for _, envVar := range containers[i].Env {
Expand Down
55 changes: 49 additions & 6 deletions pkg/controller/utils/component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

Expand Down Expand Up @@ -603,7 +604,7 @@ var _ = Describe("Component handler tests", func() {
},
}
Expect(c.Create(ctx, installation)).To(BeNil())
Expect(ensureTLSCiphers(ctx, obj, c)).To(BeNil())
Expect(ensureTLSCiphers(obj, &installation.Spec)).To(BeNil())

var containers []corev1.Container
switch o := obj.(type) {
Expand Down Expand Up @@ -693,7 +694,7 @@ var _ = Describe("Component handler tests", func() {
)
})
DescribeTable("ensuring ImagePullPolicy is set", func(obj client.Object) {
modifyPodSpec(obj, setImagePullPolicy)
modifyPodSpec(obj, func(p *corev1.PodSpec) { setImagePullPolicy(p, nil) })

switch o := obj.(type) {
case *apps.Deployment:
Expand Down Expand Up @@ -742,6 +743,30 @@ var _ = Describe("Component handler tests", func() {
),
)

Describe("setImagePullPolicy", func() {
It("fills missing policies with IfNotPresent and leaves explicit policies alone when no policy is configured", func() {
ps := &corev1.PodSpec{
Containers: []corev1.Container{{Image: "a"}, {Image: "b", ImagePullPolicy: corev1.PullAlways}},
InitContainers: []corev1.Container{{Image: "init"}},
}
setImagePullPolicy(ps, nil)
Expect(ps.Containers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent))
Expect(ps.Containers[1].ImagePullPolicy).To(Equal(corev1.PullAlways), "should not override an explicitly set policy")
Expect(ps.InitContainers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent))
})

It("overrides every container's policy when a policy is configured on the Installation", func() {
ps := &corev1.PodSpec{
Containers: []corev1.Container{{Image: "a"}, {Image: "b", ImagePullPolicy: corev1.PullAlways}},
InitContainers: []corev1.Container{{Image: "init", ImagePullPolicy: corev1.PullAlways}},
}
setImagePullPolicy(ps, ptr.To(corev1.PullNever))
Expect(ps.Containers[0].ImagePullPolicy).To(Equal(corev1.PullNever))
Expect(ps.Containers[1].ImagePullPolicy).To(Equal(corev1.PullNever), "configured policy must win over renderer defaults")
Expect(ps.InitContainers[0].ImagePullPolicy).To(Equal(corev1.PullNever))
})
})

DescribeTable("ensuring os node selectors", func(component render.Component, key client.ObjectKey, obj client.Object, expectedNodeSelectors map[string]string) {
Expect(handler.CreateOrUpdateOrDelete(ctx, component, sm)).ShouldNot(HaveOccurred())
Expect(c.Get(ctx, key, obj)).ShouldNot(HaveOccurred())
Expand Down Expand Up @@ -2190,7 +2215,15 @@ var _ = Describe("Mocked client Component handler tests", func() {
objs: []client.Object{baseNP},
}

// Two Get calls are issued up-front to load the InstallationSpec
// (one for the Installation, one for the overlay).
installationGets := func() {
mc.Info = append(mc.Info, mockReturn{Method: "Get", Return: nil})
mc.Info = append(mc.Info, mockReturn{Method: "Get", Return: nil})
}

It("NetworkPolicy updates are omitted if there is no change", func() {
installationGets()
mc.Info = append(mc.Info, mockReturn{
Method: "Get",
Return: nil,
Expand All @@ -2199,7 +2232,7 @@ var _ = Describe("Mocked client Component handler tests", func() {

err := handler.CreateOrUpdateOrDelete(ctx, fc, nil)
Expect(err).To(BeNil())
Expect(mc.Index).To(Equal(1))
Expect(mc.Index).To(Equal(3))
})

It("NetworkPolicy updates are applied if there is a change", func() {
Expand All @@ -2211,6 +2244,7 @@ var _ = Describe("Mocked client Component handler tests", func() {
}
}

installationGets()
mc.Info = append(mc.Info, mockReturn{
Method: "Get",
Return: nil,
Expand All @@ -2225,7 +2259,7 @@ var _ = Describe("Mocked client Component handler tests", func() {

err := handler.CreateOrUpdateOrDelete(ctx, fc, nil)
Expect(err).To(BeNil())
Expect(mc.Index).To(Equal(2))
Expect(mc.Index).To(Equal(4))
})
})

Expand All @@ -2246,7 +2280,15 @@ var _ = Describe("Mocked client Component handler tests", func() {
objs: []client.Object{baseTier},
}

// Two Get calls are issued up-front to load the InstallationSpec
// (one for the Installation, one for the overlay).
installationGets := func() {
mc.Info = append(mc.Info, mockReturn{Method: "Get", Return: nil})
mc.Info = append(mc.Info, mockReturn{Method: "Get", Return: nil})
}

It("Tier updates are omitted if there is no change", func() {
installationGets()
mc.Info = append(mc.Info, mockReturn{
Method: "Get",
Return: nil,
Expand All @@ -2255,7 +2297,7 @@ var _ = Describe("Mocked client Component handler tests", func() {

err := handler.CreateOrUpdateOrDelete(ctx, fc, nil)
Expect(err).To(BeNil())
Expect(mc.Index).To(Equal(1))
Expect(mc.Index).To(Equal(3))
})

It("Tier updates are applied if there is a change", func() {
Expand All @@ -2268,6 +2310,7 @@ var _ = Describe("Mocked client Component handler tests", func() {
}
}

installationGets()
mc.Info = append(mc.Info, mockReturn{
Method: "Get",
Return: nil,
Expand All @@ -2282,7 +2325,7 @@ var _ = Describe("Mocked client Component handler tests", func() {

err := handler.CreateOrUpdateOrDelete(ctx, fc, nil)
Expect(err).To(BeNil())
Expect(mc.Index).To(Equal(2))
Expect(mc.Index).To(Equal(4))
})
})
})
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/utils/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"reflect"

v1 "k8s.io/api/core/v1"
"k8s.io/utils/ptr"

operatorv1 "github.com/tigera/operator/api/v1"
)
Expand Down Expand Up @@ -60,6 +61,11 @@ func OverrideInstallationSpec(cfg, override operatorv1.InstallationSpec) operato
copy(inst.ImagePullSecrets, override.ImagePullSecrets)
}

switch compareFields(inst.ImagePullPolicy, override.ImagePullPolicy) {
case BOnlySet, Different:
inst.ImagePullPolicy = ptr.To(*override.ImagePullPolicy)
}

switch compareFields(inst.KubernetesProvider, override.KubernetesProvider) {
case BOnlySet, Different:
inst.KubernetesProvider = override.KubernetesProvider
Expand Down
19 changes: 19 additions & 0 deletions pkg/controller/utils/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/ptr"

opv1 "github.com/tigera/operator/api/v1"
"github.com/tigera/operator/test"
Expand Down Expand Up @@ -115,6 +116,24 @@ var _ = Describe("Installation merge tests", func() {
Entry("Both set not matching", []v1.LocalObjectReference{{Name: "pull-secret"}}, []v1.LocalObjectReference{{Name: "other-pull-secret"}}, []v1.LocalObjectReference{{Name: "other-pull-secret"}}),
)

DescribeTable("merge ImagePullPolicy", func(main, second, expect *v1.PullPolicy) {
m := opv1.InstallationSpec{ImagePullPolicy: main}
s := opv1.InstallationSpec{ImagePullPolicy: second}
inst := OverrideInstallationSpec(m, s)
if expect == nil {
Expect(inst.ImagePullPolicy).To(BeNil())
} else {
Expect(inst.ImagePullPolicy).NotTo(BeNil())
Expect(*inst.ImagePullPolicy).To(Equal(*expect))
}
},
Entry("Both unset", nil, nil, nil),
Entry("Main only set", ptr.To(v1.PullIfNotPresent), nil, ptr.To(v1.PullIfNotPresent)),
Entry("Second only set", nil, ptr.To(v1.PullAlways), ptr.To(v1.PullAlways)),
Entry("Both set equal", ptr.To(v1.PullIfNotPresent), ptr.To(v1.PullIfNotPresent), ptr.To(v1.PullIfNotPresent)),
Entry("Both set not matching", ptr.To(v1.PullAlways), ptr.To(v1.PullNever), ptr.To(v1.PullNever)),
)

DescribeTable("merge KubernetesProvider", func(main, second, expect *opv1.Provider) {
m := opv1.InstallationSpec{}
s := opv1.InstallationSpec{}
Expand Down
22 changes: 22 additions & 0 deletions pkg/imports/crds/operator/operator.tigera.io_installations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7184,6 +7184,17 @@ spec:
`<registry><imagePath>/<imagePrefix><imageName>:<image-tag>`
This option allows configuring the `<imagePrefix>` portion of the above format.
type: string
imagePullPolicy:
description: |-
ImagePullPolicy is the pull policy applied to containers in pods rendered by the operator
that do not explicitly set their own pull policy. If unset, defaults to IfNotPresent.
This is useful in air-gapped environments where images are pre-loaded onto nodes and
must not be re-pulled from a remote registry.
enum:
- Always
- IfNotPresent
- Never
type: string
imagePullSecrets:
description: |-
ImagePullSecrets is an array of references to container registry pull secrets to use. These are
Expand Down Expand Up @@ -16524,6 +16535,17 @@ spec:
`<registry><imagePath>/<imagePrefix><imageName>:<image-tag>`
This option allows configuring the `<imagePrefix>` portion of the above format.
type: string
imagePullPolicy:
description: |-
ImagePullPolicy is the pull policy applied to containers in pods rendered by the operator
that do not explicitly set their own pull policy. If unset, defaults to IfNotPresent.
This is useful in air-gapped environments where images are pre-loaded onto nodes and
must not be re-pulled from a remote registry.
enum:
- Always
- IfNotPresent
- Never
type: string
imagePullSecrets:
description: |-
ImagePullSecrets is an array of references to container registry pull secrets to use. These are
Expand Down
Loading
Loading