Skip to content

Commit e22f35d

Browse files
joelanfordjoelanford
committed
wip: only override deployment resources when explicitly defined in subscription.spec.config
Signed-off-by: Joe Lanford <[email protected]>
1 parent 23904fb commit e22f35d

File tree

41 files changed

+1136
-494
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+1136
-494
lines changed

Diff for: ‎Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ run-console-local:
236236
uninstall:
237237
@echo Uninstalling OLM:
238238
- kubectl delete -f deploy/upstream/quickstart/crds.yaml
239-
- kubectl delete -f deploy/upstream/quickstart/olm.yam
239+
- kubectl delete -f deploy/upstream/quickstart/olm.yaml
240240
- kubectl delete catalogsources.operators.coreos.com
241241
- kubectl delete clusterserviceversions.operators.coreos.com
242242
- kubectl delete installplans.operators.coreos.com

Diff for: ‎go.mod

+10-8
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,18 @@ require (
4141
google.golang.org/grpc v1.30.0
4242
gopkg.in/yaml.v2 v2.3.0
4343
helm.sh/helm/v3 v3.1.0-rc.1.0.20201215141456-e71d38b414eb
44-
k8s.io/api v0.20.0
45-
k8s.io/apiextensions-apiserver v0.20.0
46-
k8s.io/apimachinery v0.20.0
47-
k8s.io/apiserver v0.20.0
48-
k8s.io/client-go v0.20.0
49-
k8s.io/code-generator v0.20.0
50-
k8s.io/component-base v0.20.0
44+
k8s.io/api v0.20.1
45+
k8s.io/apiextensions-apiserver v0.20.1
46+
k8s.io/apimachinery v0.20.1
47+
k8s.io/apiserver v0.20.1
48+
k8s.io/client-go v0.20.1
49+
k8s.io/code-generator v0.20.1
50+
k8s.io/component-base v0.20.1
5151
k8s.io/klog v1.0.0
5252
k8s.io/kube-aggregator v0.20.0
5353
k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd
5454
rsc.io/letsencrypt v0.0.3 // indirect
55-
sigs.k8s.io/controller-runtime v0.7.0
55+
sigs.k8s.io/controller-runtime v0.8.0
5656
sigs.k8s.io/controller-tools v0.4.1
5757
sigs.k8s.io/kind v0.7.0
5858
)
@@ -64,6 +64,8 @@ replace (
6464
github.com/openshift/api => github.com/openshift/api v0.0.0-20200331152225-585af27e34fd // release-4.5
6565
github.com/openshift/client-go => github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0 // release-4.5
6666

67+
github.com/operator-framework/api => github.com/joelanford/api v0.0.0-20210216180012-67617936c73e
68+
6769
// pinned because latest etcd does not yet work with the latest grpc version (1.30.0)
6870
go.etcd.io/etcd => go.etcd.io/etcd v0.5.0-alpha.5.0.20200520232829-54ba9589114f
6971
google.golang.org/grpc => google.golang.org/grpc v1.27.0

Diff for: ‎go.sum

+16-42
Large diffs are not rendered by default.

Diff for: ‎pkg/controller/operators/olm/overrides/config.go

+9-7
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, err error) {
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, 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)
@@ -29,12 +29,14 @@ func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOve
2929
return
3030
}
3131

32-
envVarOverrides = owner.Spec.Config.Env
33-
volumeOverrides = owner.Spec.Config.Volumes
34-
volumeMountOverrides = owner.Spec.Config.VolumeMounts
35-
tolerationOverrides = owner.Spec.Config.Tolerations
36-
resourcesOverride = owner.Spec.Config.Resources
37-
nodeSelectorOverride = owner.Spec.Config.NodeSelector
32+
if owner.Spec.Config != nil {
33+
envVarOverrides = owner.Spec.Config.Env
34+
volumeOverrides = owner.Spec.Config.Volumes
35+
volumeMountOverrides = owner.Spec.Config.VolumeMounts
36+
tolerationOverrides = owner.Spec.Config.Tolerations
37+
resourcesOverride = owner.Spec.Config.Resources
38+
nodeSelectorOverride = owner.Spec.Config.NodeSelector
39+
}
3840

3941
return
4042
}

Diff for: ‎pkg/controller/operators/olm/overrides/inject/inject.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -194,14 +194,18 @@ func findToleration(tolerations []corev1.Toleration, toleration corev1.Toleratio
194194
// into given podSpec
195195
//
196196
// If podSpec already defines Resources, it will be overwritten
197-
func InjectResourcesIntoDeployment(podSpec *corev1.PodSpec, resources corev1.ResourceRequirements) error {
197+
func InjectResourcesIntoDeployment(podSpec *corev1.PodSpec, resources *corev1.ResourceRequirements) error {
198198
if podSpec == nil {
199199
return errors.New("no pod spec provided")
200200
}
201201

202+
if resources == nil {
203+
return nil
204+
}
205+
202206
for i := range podSpec.Containers {
203207
container := &podSpec.Containers[i]
204-
container.Resources = resources
208+
container.Resources = *resources
205209
}
206210

207211
return nil

Diff for: ‎pkg/controller/operators/olm/overrides/inject/inject_test.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -631,25 +631,25 @@ func TestInjectResourcesIntoDeployment(t *testing.T) {
631631
tests := []struct {
632632
name string
633633
podSpec *corev1.PodSpec
634-
resources corev1.ResourceRequirements
634+
resources *corev1.ResourceRequirements
635635
expected *corev1.PodSpec
636636
}{
637637
{
638-
// PodSpec has one container and empty resources
639-
// Expected: Resources will remain empty
640-
name: "WithEmptyResources",
638+
// PodSpec has one container and existing resources
639+
// Expected: PodSpec resources will remain untouched
640+
name: "WithNilResources",
641641
podSpec: &corev1.PodSpec{
642642
Containers: []corev1.Container{
643643
corev1.Container{
644-
Resources: corev1.ResourceRequirements{},
644+
Resources: defaultResources,
645645
},
646646
},
647647
},
648-
resources: corev1.ResourceRequirements{},
648+
resources: nil,
649649
expected: &corev1.PodSpec{
650650
Containers: []corev1.Container{
651651
corev1.Container{
652-
Resources: corev1.ResourceRequirements{},
652+
Resources: defaultResources,
653653
},
654654
},
655655
},
@@ -665,7 +665,7 @@ func TestInjectResourcesIntoDeployment(t *testing.T) {
665665
},
666666
},
667667
},
668-
resources: defaultResources,
668+
resources: &defaultResources,
669669
expected: &corev1.PodSpec{
670670
Containers: []corev1.Container{
671671
corev1.Container{
@@ -686,7 +686,7 @@ func TestInjectResourcesIntoDeployment(t *testing.T) {
686686
},
687687
},
688688
},
689-
resources: corev1.ResourceRequirements{},
689+
resources: &corev1.ResourceRequirements{},
690690
expected: &corev1.PodSpec{
691691
Containers: []corev1.Container{
692692
corev1.Container{

Diff for: ‎test/e2e/subscription_e2e_test.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -1188,7 +1188,7 @@ var _ = Describe("Subscription", func() {
11881188
Operator: corev1.TolerationOpEqual,
11891189
},
11901190
}
1191-
podResources := corev1.ResourceRequirements{
1191+
podResources := &corev1.ResourceRequirements{
11921192
Limits: corev1.ResourceList{
11931193
corev1.ResourceCPU: resource.MustParse("100m"),
11941194
},
@@ -1198,7 +1198,7 @@ var _ = Describe("Subscription", func() {
11981198
},
11991199
}
12001200

1201-
podConfig := v1alpha1.SubscriptionConfig{
1201+
podConfig := &v1alpha1.SubscriptionConfig{
12021202
Env: podEnv,
12031203
Volumes: podVolumes,
12041204
VolumeMounts: podVolumeMounts,
@@ -1257,7 +1257,7 @@ var _ = Describe("Subscription", func() {
12571257
"foo": "bar",
12581258
}
12591259

1260-
podConfig := v1alpha1.SubscriptionConfig{
1260+
podConfig := &v1alpha1.SubscriptionConfig{
12611261
NodeSelector: podNodeSelector,
12621262
}
12631263

@@ -2463,7 +2463,7 @@ func checkDeploymentHasPodConfigNodeSelector(t GinkgoTInterface, client operator
24632463
return nil
24642464
}
24652465

2466-
func checkDeploymentWithPodConfiguration(t GinkgoTInterface, client operatorclient.ClientInterface, csv *v1alpha1.ClusterServiceVersion, envVar []corev1.EnvVar, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, tolerations []corev1.Toleration, resources corev1.ResourceRequirements) {
2466+
func checkDeploymentWithPodConfiguration(t GinkgoTInterface, client operatorclient.ClientInterface, csv *v1alpha1.ClusterServiceVersion, envVar []corev1.EnvVar, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, tolerations []corev1.Toleration, resources *corev1.ResourceRequirements) {
24672467
resolver := install.StrategyResolver{}
24682468

24692469
strategy, err := resolver.UnmarshalStrategy(csv.Spec.InstallStrategy)
@@ -2524,10 +2524,10 @@ func checkDeploymentWithPodConfiguration(t GinkgoTInterface, client operatorclie
25242524
return
25252525
}
25262526

2527-
findResources := func(existingResource corev1.ResourceRequirements, podResource corev1.ResourceRequirements) (foundResource *corev1.ResourceRequirements, found bool) {
2527+
findResources := func(existingResource *corev1.ResourceRequirements, podResource *corev1.ResourceRequirements) (foundResource *corev1.ResourceRequirements, found bool) {
25282528
if reflect.DeepEqual(existingResource, podResource) {
25292529
found = true
2530-
foundResource = &podResource
2530+
foundResource = podResource
25312531
}
25322532

25332533
return
@@ -2548,7 +2548,7 @@ func checkDeploymentWithPodConfiguration(t GinkgoTInterface, client operatorclie
25482548
require.Equalf(t, v.MountPath, existing.MountPath, "VolumeMount MountPath does not match %s=%s", v.Name, v.MountPath)
25492549
}
25502550

2551-
existing, found := findResources(container.Resources, resources)
2551+
existing, found := findResources(&container.Resources, resources)
25522552
require.Truef(t, found, "Resources not injected. Resource=%v", resources)
25532553
require.NotNil(t, existing)
25542554
require.Equalf(t, *existing, resources, "Resource=%v does not match expected Resource=%v", existing, resources)

Diff for: ‎vendor/github.com/operator-framework/api/crds/operators.coreos.com_operatorconditions.yaml

+4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎vendor/github.com/operator-framework/api/crds/zz_defs.go

+8-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎vendor/github.com/operator-framework/api/pkg/operators/v1/operatorcondition_types.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/subscription_types.go

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/zz_generated.deepcopy.go

+10-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎vendor/github.com/operator-framework/api/pkg/validation/errors/error.go

-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎vendor/github.com/operator-framework/api/pkg/validation/internal/operatorhub.go

+9-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)