Skip to content

Commit 1770db7

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #387 from anik120/psm-4.10-backport
[release-4.10] OCPBUGS-1525: Package Server Manager should enforce expected csv values
2 parents 8892eeb + 5f3f8e2 commit 1770db7

File tree

11 files changed

+224
-140
lines changed

11 files changed

+224
-140
lines changed

‎go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/openshift/operator-framework-olm
33
go 1.17
44

55
require (
6+
github.com/blang/semver/v4 v4.0.0
67
github.com/go-bindata/go-bindata/v3 v3.1.3
78
github.com/go-logr/logr v0.4.0
89
github.com/golang/mock v1.6.0
@@ -57,8 +58,7 @@ require (
5758
github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535 // indirect
5859
github.com/beorn7/perks v1.0.1 // indirect
5960
github.com/blang/semver v3.5.1+incompatible // indirect
60-
github.com/blang/semver/v4 v4.0.0 // indirect
61-
github.com/cespare/xxhash/v2 v2.1.1 // indirect
61+
github.com/cespare/xxhash/v2 v2.1.2 // indirect
6262
github.com/containerd/cgroups v1.0.3 // indirect
6363
github.com/containerd/containerd v1.5.13 // indirect
6464
github.com/containerd/continuity v0.1.0 // indirect

‎go.sum

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,9 @@ github.com/certifi/gocertifi v0.0.0-20191021191039-0944d244cd40/go.mod h1:sGbDF6
205205
github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054/go.mod h1:sGbDF6GwGcLpkNXPUTkMRoywsNa/ol15pxFe6ERfguA=
206206
github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=
207207
github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
208-
github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY=
209208
github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
209+
github.com/cespare/xxhash/v2 v2.1.2 h1:YRXhKfTDauu4ajMg1TPgFO5jnlC2HCbmLXMcTG5cbYE=
210+
github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
210211
github.com/chai2010/gettext-go v0.0.0-20160711120539-c6fed771bfd5/go.mod h1:/iP1qXHoty45bqomnu2LM+VVyAEdWN+vtSHGlQgyxbw=
211212
github.com/checkpoint-restore/go-criu/v4 v4.1.0/go.mod h1:xUQBLp4RLc5zJtWY++yjOoMoB5lihDt7fai+75m+rGw=
212213
github.com/checkpoint-restore/go-criu/v5 v5.0.0/go.mod h1:cfwC0EG7HMUenopBsUf9d89JlCLQIfgVcNsNN0t6T2M=

‎pkg/package-server-manager/config.go

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1212
"k8s.io/apimachinery/pkg/util/intstr"
1313
"k8s.io/utils/pointer"
14+
15+
"github.com/openshift/operator-framework-olm/pkg/manifests"
1416
)
1517

1618
func getReplicas(ha bool) int32 {
@@ -61,61 +63,91 @@ func getTopologyModeFromInfra(infra *configv1.Infrastructure) bool {
6163
}
6264

6365
// ensureCSV is responsible for ensuring the state of the @csv ClusterServiceVersion custom
66+
// resource matches that of the codified defaults and high availability configurations, where
67+
// codified defaults are defined by the csv returned by the manifests.NewPackageServerCSV
68+
// function.
69+
func ensureCSV(log logr.Logger, image string, csv *olmv1alpha1.ClusterServiceVersion, highlyAvailableMode bool) (bool, error) {
70+
expectedCSV, err := manifests.NewPackageServerCSV(
71+
manifests.WithName(csv.Name),
72+
manifests.WithNamespace(csv.Namespace),
73+
manifests.WithImage(image),
74+
)
75+
if err != nil {
76+
return false, err
77+
}
78+
79+
ensureCSVHighAvailability(image, expectedCSV, highlyAvailableMode)
80+
81+
var modified bool
82+
83+
for k, v := range expectedCSV.GetLabels() {
84+
if csv.GetLabels() == nil {
85+
csv.SetLabels(make(map[string]string))
86+
}
87+
if vv, ok := csv.GetLabels()[k]; !ok || vv != v {
88+
log.Info("setting expected label", "key", k, "value", v)
89+
csv.ObjectMeta.Labels[k] = v
90+
modified = true
91+
}
92+
}
93+
94+
for k, v := range expectedCSV.GetAnnotations() {
95+
if csv.GetAnnotations() == nil {
96+
csv.SetAnnotations(make(map[string]string))
97+
}
98+
if vv, ok := csv.GetAnnotations()[k]; !ok || vv != v {
99+
log.Info("setting expected annotation", "key", k, "value", v)
100+
csv.ObjectMeta.Annotations[k] = v
101+
modified = true
102+
}
103+
}
104+
105+
if !reflect.DeepEqual(expectedCSV.Spec, csv.Spec) {
106+
log.Info("updating csv spec")
107+
csv.Spec = expectedCSV.Spec
108+
modified = true
109+
}
110+
111+
if modified {
112+
log.V(3).Info("csv has been modified")
113+
}
114+
115+
return modified, err
116+
}
117+
118+
// ensureCSVHighAvailability is responsible for ensuring the state of the @csv ClusterServiceVersion custom
64119
// resource matches the expected state based on any high availability expectations being exposed.
65-
func ensureCSV(log logr.Logger, image string, csv *olmv1alpha1.ClusterServiceVersion, highlyAvailableMode bool) bool {
120+
func ensureCSVHighAvailability(image string, csv *olmv1alpha1.ClusterServiceVersion, highlyAvailableMode bool) {
66121
var modified bool
67122

68123
deploymentSpecs := csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs
69124
deployment := &deploymentSpecs[0].Spec
70125

71126
currentImage := deployment.Template.Spec.Containers[0].Image
72127
if currentImage != image {
73-
log.Info("updating the image", "old", currentImage, "new", image)
74128
deployment.Template.Spec.Containers[0].Image = image
75129
modified = true
76130
}
77131

78132
expectedReplicas := getReplicas(highlyAvailableMode)
79133
if *deployment.Replicas != expectedReplicas {
80-
log.Info("updating the replica count", "old", deployment.Replicas, "new", expectedReplicas)
81134
deployment.Replicas = pointer.Int32Ptr(expectedReplicas)
82135
modified = true
83136
}
84137

85138
expectedRolloutConfiguration := getRolloutStrategy(highlyAvailableMode)
86139
if !reflect.DeepEqual(deployment.Strategy.RollingUpdate, expectedRolloutConfiguration) {
87-
log.Info("updating the rollout strategy")
88140
deployment.Strategy.RollingUpdate = expectedRolloutConfiguration
89141
modified = true
90142
}
91143

92144
expectedAffinityConfiguration := getAntiAffinityConfig(highlyAvailableMode)
93145
if !reflect.DeepEqual(deployment.Template.Spec.Affinity, expectedAffinityConfiguration) {
94-
log.Info("updating the pod anti-affinity configuration")
95146
deployment.Template.Spec.Affinity = expectedAffinityConfiguration
96147
modified = true
97148
}
98149

99150
if modified {
100-
log.V(3).Info("csv has been modified")
101151
csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec = *deployment
102152
}
103-
104-
return modified
105-
}
106-
107-
func validateCSV(log logr.Logger, csv *olmv1alpha1.ClusterServiceVersion) bool {
108-
deploymentSpecs := csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs
109-
if len(deploymentSpecs) != 1 {
110-
log.Info("csv contains more than one or zero nested deployment specs")
111-
return false
112-
}
113-
114-
deployment := &deploymentSpecs[0].Spec
115-
if len(deployment.Template.Spec.Containers) != 1 {
116-
log.Info("csv contains more than one container")
117-
return false
118-
}
119-
120-
return true
121153
}

‎pkg/package-server-manager/controller.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controllers
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"sync"
2223

2324
"github.com/go-logr/logr"
@@ -85,11 +86,12 @@ func (r *PackageServerCSVReconciler) Reconcile(ctx context.Context, req ctrl.Req
8586
res, err := controllerutil.CreateOrUpdate(ctx, r.Client, required, func() error {
8687
return reconcileCSV(r.Log, r.Image, required, highAvailabilityMode)
8788
})
89+
90+
log.Info("reconciliation result", "res", res)
8891
if err != nil {
8992
log.Error(err, "failed to create or update the packageserver csv")
9093
return ctrl.Result{}, nil
9194
}
92-
log.Info("reconciliation result", "res", res)
9395

9496
return ctrl.Result{}, nil
9597
}
@@ -98,19 +100,13 @@ func reconcileCSV(log logr.Logger, image string, csv *olmv1alpha1.ClusterService
98100
if csv.ObjectMeta.CreationTimestamp.IsZero() {
99101
log.Info("attempting to create the packageserver csv")
100102
}
101-
if !validateCSV(log, csv) {
102-
log.Info("updating invalid csv to use the default configuration")
103-
tmp, err := manifests.NewPackageServerCSV(
104-
manifests.WithName(csv.Name),
105-
manifests.WithNamespace(csv.Namespace),
106-
manifests.WithImage(image),
107-
)
108-
if err != nil {
109-
return err
110-
}
111-
csv.Spec = tmp.Spec
103+
104+
modified, err := ensureCSV(log, image, csv, highAvailabilityMode)
105+
if err != nil {
106+
return fmt.Errorf("error ensuring CSV: %v", err)
112107
}
113-
if !ensureCSV(log, image, csv, highAvailabilityMode) {
108+
109+
if !modified {
114110
log.V(3).Info("no further updates are necessary to the packageserver csv")
115111
}
116112

0 commit comments

Comments
 (0)