Skip to content

Commit d9e3836

Browse files
tmshorthoris233kevinrizza
authored
Support EnvFrom (#3193)
* Bug: Enable SubscriptionConfig EnvFrom field Problem: EnvFrom is claimed supported in the document, but it isn't enabled from the code Solution: Wire the EnvFrom specified in the SubscriptionConfig into the deployments deployed when installing the operator. Signed-off-by: Jiaming Hu <[email protected]> * add config envFrom overlaps test case Signed-off-by: Jiaming Hu <[email protected]> * Fix test changes for envfrom Signed-off-by: kevinrizza <[email protected]> * Cleanup fromEnv code Signed-off-by: Todd Short <[email protected]> * Use reflect to compare fromEnv Signed-off-by: Todd Short <[email protected]> --------- Signed-off-by: Jiaming Hu <[email protected]> Signed-off-by: kevinrizza <[email protected]> Signed-off-by: Todd Short <[email protected]> Co-authored-by: Jiaming Hu <[email protected]> Co-authored-by: kevinrizza <[email protected]>
1 parent 275d62e commit d9e3836

File tree

4 files changed

+231
-2
lines changed

4 files changed

+231
-2
lines changed

‎pkg/controller/operators/olm/overrides/config.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ type operatorConfig struct {
1616
logger *logrus.Logger
1717
}
1818

19-
func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOverrides []corev1.EnvVar, volumeOverrides []corev1.Volume, volumeMountOverrides []corev1.VolumeMount, tolerationOverrides []corev1.Toleration, resourcesOverride *corev1.ResourceRequirements, nodeSelectorOverride map[string]string, affinity *corev1.Affinity, annotations map[string]string, err error) {
19+
func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOverrides []corev1.EnvVar, envFromOverrides []corev1.EnvFromSource, volumeOverrides []corev1.Volume, volumeMountOverrides []corev1.VolumeMount, tolerationOverrides []corev1.Toleration, resourcesOverride *corev1.ResourceRequirements, nodeSelectorOverride map[string]string, affinity *corev1.Affinity, annotations map[string]string, err error) {
2020
list, listErr := o.lister.OperatorsV1alpha1().SubscriptionLister().Subscriptions(ownerCSV.GetNamespace()).List(labels.Everything())
2121
if listErr != nil {
2222
err = fmt.Errorf("failed to list subscription namespace=%s - %v", ownerCSV.GetNamespace(), listErr)
@@ -35,6 +35,7 @@ func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOve
3535
}
3636

3737
envVarOverrides = owner.Spec.Config.Env
38+
envFromOverrides = owner.Spec.Config.EnvFrom
3839
volumeOverrides = owner.Spec.Config.Volumes
3940
volumeMountOverrides = owner.Spec.Config.VolumeMounts
4041
tolerationOverrides = owner.Spec.Config.Tolerations

‎pkg/controller/operators/olm/overrides/initializer.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment
4545
var envVarOverrides, proxyEnvVar, merged []corev1.EnvVar
4646
var err error
4747

48-
envVarOverrides, volumeOverrides, volumeMountOverrides, tolerationOverrides, resourcesOverride, nodeSelectorOverride, affinity, annotations, err := d.config.GetConfigOverrides(ownerCSV)
48+
envVarOverrides, envFromOverrides, volumeOverrides, volumeMountOverrides, tolerationOverrides, resourcesOverride, nodeSelectorOverride, affinity, annotations, err := d.config.GetConfigOverrides(ownerCSV)
4949
if err != nil {
5050
err = fmt.Errorf("failed to get subscription pod configuration - %v", err)
5151
return err
@@ -72,6 +72,10 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment
7272
return fmt.Errorf("failed to inject proxy env variable(s) into deployment spec name=%s - %v", deployment.Name, err)
7373
}
7474

75+
if err := inject.InjectEnvFromIntoDeployment(podSpec, envFromOverrides); err != nil {
76+
return fmt.Errorf("failed to inject envFrom variable(s) into deployment spec name=%s - %v", deployment.Name, err)
77+
}
78+
7579
if err = inject.InjectVolumesIntoDeployment(podSpec, volumeOverrides); err != nil {
7680
return fmt.Errorf("failed to inject volume(s) into deployment spec name=%s - %v", deployment.Name, err)
7781
}

‎pkg/controller/operators/olm/overrides/inject/inject.go

+39
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,45 @@ func mergeEnvVars(containerEnvVars []corev1.EnvVar, newEnvVars []corev1.EnvVar)
6262
return merged
6363
}
6464

65+
// InjectEnvFromIntoDeployment injects the envFrom variables
66+
// into the container(s) of the given PodSpec.
67+
//
68+
// If any Container in PodSpec already defines an envFrom variable
69+
// as any of the provided envFrom then it will be overwritten.
70+
func InjectEnvFromIntoDeployment(podSpec *corev1.PodSpec, envFromVars []corev1.EnvFromSource) error {
71+
if podSpec == nil {
72+
return errors.New("no pod spec provided")
73+
}
74+
75+
for i := range podSpec.Containers {
76+
container := &podSpec.Containers[i]
77+
container.EnvFrom = mergeEnvFromVars(container.EnvFrom, envFromVars)
78+
}
79+
80+
return nil
81+
}
82+
83+
func mergeEnvFromVars(containerEnvFromVars []corev1.EnvFromSource, newEnvFromVars []corev1.EnvFromSource) []corev1.EnvFromSource {
84+
merged := containerEnvFromVars
85+
86+
for _, newEnvFromVar := range newEnvFromVars {
87+
if !findEnvFromVar(containerEnvFromVars, newEnvFromVar) {
88+
merged = append(merged, newEnvFromVar)
89+
}
90+
}
91+
92+
return merged
93+
}
94+
95+
func findEnvFromVar(envFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) bool {
96+
for i := range envFromVar {
97+
if reflect.DeepEqual(envFromVar[i], newEnvFromVar) {
98+
return true
99+
}
100+
}
101+
return false
102+
}
103+
65104
// InjectVolumesIntoDeployment injects the provided Volumes
66105
// into the container(s) of the given PodSpec.
67106
//

‎pkg/controller/operators/olm/overrides/inject/inject_test.go

+185
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,26 @@ var (
3030
},
3131
}
3232

33+
defaultEnvFromVars = []corev1.EnvFromSource{
34+
{
35+
Prefix: "test",
36+
},
37+
{
38+
ConfigMapRef: &corev1.ConfigMapEnvSource{
39+
LocalObjectReference: corev1.LocalObjectReference{
40+
Name: "configmapForTest",
41+
},
42+
},
43+
},
44+
{
45+
SecretRef: &corev1.SecretEnvSource{
46+
LocalObjectReference: corev1.LocalObjectReference{
47+
Name: "secretForTest",
48+
},
49+
},
50+
},
51+
}
52+
3353
defaultVolumeMounts = []corev1.VolumeMount{
3454
{
3555
Name: "foo",
@@ -526,6 +546,171 @@ func TestInjectEnvIntoDeployment(t *testing.T) {
526546
}
527547
}
528548

549+
func TestInjectEnvFromIntoDeployment(t *testing.T) {
550+
tests := []struct {
551+
name string
552+
podSpec *corev1.PodSpec
553+
envFromVar []corev1.EnvFromSource
554+
expected *corev1.PodSpec
555+
}{
556+
{
557+
// PodSpec has one container and `EnvFrom` is empty.
558+
// Expected: All env variable(s) specified are injected.
559+
name: "WithContainerHasNoEnvFromVar",
560+
podSpec: &corev1.PodSpec{
561+
Containers: []corev1.Container{
562+
{},
563+
},
564+
},
565+
envFromVar: defaultEnvFromVars,
566+
expected: &corev1.PodSpec{
567+
Containers: []corev1.Container{
568+
{
569+
EnvFrom: defaultEnvFromVars,
570+
},
571+
},
572+
},
573+
},
574+
{
575+
// PodSpec has one container and it has overlapping envFrom var(s).
576+
// Expected: existing duplicate env vars won't be appended in the envFrom.
577+
name: "WithContainerHasOverlappingEnvFromVar",
578+
podSpec: &corev1.PodSpec{
579+
Containers: []corev1.Container{
580+
{
581+
EnvFrom: []corev1.EnvFromSource{
582+
{
583+
Prefix: "test",
584+
},
585+
{
586+
ConfigMapRef: &corev1.ConfigMapEnvSource{
587+
LocalObjectReference: corev1.LocalObjectReference{
588+
Name: "configmapForTest",
589+
},
590+
},
591+
},
592+
{
593+
SecretRef: &corev1.SecretEnvSource{
594+
LocalObjectReference: corev1.LocalObjectReference{
595+
Name: "secretForTest",
596+
},
597+
},
598+
},
599+
},
600+
},
601+
},
602+
},
603+
envFromVar: defaultEnvFromVars,
604+
expected: &corev1.PodSpec{
605+
Containers: []corev1.Container{
606+
{
607+
EnvFrom: []corev1.EnvFromSource{
608+
{
609+
Prefix: "test",
610+
},
611+
{
612+
ConfigMapRef: &corev1.ConfigMapEnvSource{
613+
LocalObjectReference: corev1.LocalObjectReference{
614+
Name: "configmapForTest",
615+
},
616+
},
617+
},
618+
{
619+
SecretRef: &corev1.SecretEnvSource{
620+
LocalObjectReference: corev1.LocalObjectReference{
621+
Name: "secretForTest",
622+
},
623+
},
624+
},
625+
},
626+
},
627+
},
628+
},
629+
},
630+
{
631+
// PodSpec has one container and it has non overlapping envFrom var(s).
632+
// Expected: existing non overlapping env vars are intact.
633+
name: "WithContainerHasNonOverlappingEnvFromVar",
634+
podSpec: &corev1.PodSpec{
635+
Containers: []corev1.Container{
636+
{
637+
EnvFrom: []corev1.EnvFromSource{
638+
{
639+
Prefix: "foo",
640+
},
641+
},
642+
},
643+
},
644+
},
645+
envFromVar: defaultEnvFromVars,
646+
expected: &corev1.PodSpec{
647+
Containers: []corev1.Container{
648+
{
649+
EnvFrom: append([]corev1.EnvFromSource{
650+
{
651+
Prefix: "foo",
652+
},
653+
}, defaultEnvFromVars...),
654+
},
655+
},
656+
},
657+
},
658+
{
659+
// PodSpec has more than one container(s)
660+
// Expected: All container(s) should be updated as expected.
661+
name: "WithMultipleContainers",
662+
podSpec: &corev1.PodSpec{
663+
Containers: []corev1.Container{
664+
{},
665+
{
666+
EnvFrom: []corev1.EnvFromSource{
667+
{
668+
Prefix: "foo",
669+
},
670+
},
671+
},
672+
{
673+
EnvFrom: []corev1.EnvFromSource{
674+
{
675+
Prefix: "bar",
676+
},
677+
},
678+
},
679+
},
680+
},
681+
envFromVar: defaultEnvFromVars,
682+
expected: &corev1.PodSpec{
683+
Containers: []corev1.Container{
684+
{
685+
EnvFrom: defaultEnvFromVars,
686+
},
687+
{
688+
EnvFrom: append([]corev1.EnvFromSource{
689+
{
690+
Prefix: "foo",
691+
},
692+
}, defaultEnvFromVars...),
693+
},
694+
{
695+
EnvFrom: append([]corev1.EnvFromSource{
696+
{
697+
Prefix: "bar",
698+
},
699+
}, defaultEnvFromVars...),
700+
},
701+
},
702+
},
703+
},
704+
}
705+
706+
for _, tt := range tests {
707+
t.Run(tt.name, func(t *testing.T) {
708+
inject.InjectEnvFromIntoDeployment(tt.podSpec, tt.envFromVar)
709+
assert.Equal(t, tt.expected, tt.podSpec)
710+
})
711+
}
712+
}
713+
529714
func TestInjectTolerationsIntoDeployment(t *testing.T) {
530715
tests := []struct {
531716
name string

0 commit comments

Comments
 (0)