Skip to content

Commit bf9eeb1

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1884 from benluddy/service-updates
Bug 1898500: Support InstallPlan steps upgrading existing ClusterIP Services.
2 parents 1259808 + 0f1f2dc commit bf9eeb1

File tree

3 files changed

+290
-6
lines changed

3 files changed

+290
-6
lines changed

‎pkg/lib/operatorclient/service.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,22 @@ func (c *Client) DeleteService(namespace, name string, options *metav1.DeleteOpt
2828
// UpdateService will update the given Service resource.
2929
func (c *Client) UpdateService(service *v1.Service) (*v1.Service, error) {
3030
klog.V(4).Infof("[UPDATE Service]: %s", service.GetName())
31-
oldSa, err := c.GetService(service.GetNamespace(), service.GetName())
31+
old, err := c.GetService(service.GetNamespace(), service.GetName())
3232
if err != nil {
3333
return nil, err
3434
}
35-
patchBytes, err := createPatch(oldSa, service)
35+
normalized, err := cloneAndNormalizeObject(old)
36+
if err != nil {
37+
return nil, fmt.Errorf("failed to normalize existing Service resource for patch: %w", err)
38+
}
39+
if service.Spec.ClusterIP == old.Spec.ClusterIP {
40+
// Support updating to manifests that specify a
41+
// ClusterIP when its value is the same as that of the
42+
// existing Service.
43+
service = service.DeepCopy()
44+
service.Spec.ClusterIP = ""
45+
}
46+
patchBytes, err := createPatch(normalized, service)
3647
if err != nil {
3748
return nil, fmt.Errorf("error creating patch for Service: %v", err)
3849
}

‎pkg/lib/operatorclient/service_test.go

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
package operatorclient
2+
3+
import (
4+
"testing"
5+
6+
corev1 "k8s.io/api/core/v1"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"k8s.io/apimachinery/pkg/runtime/schema"
9+
"k8s.io/apimachinery/pkg/types"
10+
"k8s.io/client-go/kubernetes/fake"
11+
clienttesting "k8s.io/client-go/testing"
12+
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
func TestUpdateService(t *testing.T) {
17+
gvr := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "services"}
18+
19+
// In a test expectation, matches any single fake client action.
20+
var wildcard clienttesting.Action = clienttesting.ActionImpl{Verb: "wildcard!"}
21+
22+
for _, tc := range []struct {
23+
Name string
24+
Old *corev1.Service
25+
New *corev1.Service
26+
Expected []clienttesting.Action
27+
}{
28+
{
29+
Name: "no changes",
30+
Old: &corev1.Service{
31+
ObjectMeta: metav1.ObjectMeta{
32+
Name: "name",
33+
Namespace: "namespace",
34+
},
35+
},
36+
New: &corev1.Service{
37+
ObjectMeta: metav1.ObjectMeta{
38+
Name: "name",
39+
Namespace: "namespace",
40+
},
41+
},
42+
Expected: []clienttesting.Action{
43+
wildcard,
44+
clienttesting.NewPatchAction(gvr, "namespace", "name", types.StrategicMergePatchType, []byte(`{}`)),
45+
},
46+
},
47+
{
48+
Name: "resourceversion not patched",
49+
Old: &corev1.Service{
50+
ObjectMeta: metav1.ObjectMeta{
51+
Name: "name",
52+
Namespace: "namespace",
53+
ResourceVersion: "42",
54+
},
55+
},
56+
New: &corev1.Service{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: "name",
59+
Namespace: "namespace",
60+
},
61+
},
62+
Expected: []clienttesting.Action{
63+
wildcard,
64+
clienttesting.NewPatchAction(gvr, "namespace", "name", types.StrategicMergePatchType, []byte(`{}`)),
65+
},
66+
},
67+
{
68+
Name: "clusterip not patched if omitted",
69+
Old: &corev1.Service{
70+
ObjectMeta: metav1.ObjectMeta{
71+
Name: "name",
72+
Namespace: "namespace",
73+
},
74+
Spec: corev1.ServiceSpec{
75+
ClusterIP: "1.2.3.4",
76+
},
77+
},
78+
New: &corev1.Service{
79+
ObjectMeta: metav1.ObjectMeta{
80+
Name: "name",
81+
Namespace: "namespace",
82+
},
83+
},
84+
Expected: []clienttesting.Action{
85+
wildcard,
86+
clienttesting.NewPatchAction(gvr, "namespace", "name", types.StrategicMergePatchType, []byte(`{}`)),
87+
},
88+
},
89+
{
90+
Name: "clusterip not patched if unchanged",
91+
Old: &corev1.Service{
92+
ObjectMeta: metav1.ObjectMeta{
93+
Name: "name",
94+
Namespace: "namespace",
95+
},
96+
Spec: corev1.ServiceSpec{
97+
ClusterIP: "1.2.3.4",
98+
},
99+
},
100+
New: &corev1.Service{
101+
ObjectMeta: metav1.ObjectMeta{
102+
Name: "name",
103+
Namespace: "namespace",
104+
},
105+
Spec: corev1.ServiceSpec{
106+
ClusterIP: "1.2.3.4",
107+
},
108+
},
109+
Expected: []clienttesting.Action{
110+
wildcard,
111+
clienttesting.NewPatchAction(gvr, "namespace", "name", types.StrategicMergePatchType, []byte(`{}`)),
112+
},
113+
},
114+
{
115+
Name: "clusterip patched if changed", // even though the patch will be rejected due to field immutability
116+
Old: &corev1.Service{
117+
ObjectMeta: metav1.ObjectMeta{
118+
Name: "name",
119+
Namespace: "namespace",
120+
},
121+
Spec: corev1.ServiceSpec{
122+
ClusterIP: "1.2.3.4",
123+
},
124+
},
125+
New: &corev1.Service{
126+
ObjectMeta: metav1.ObjectMeta{
127+
Name: "name",
128+
Namespace: "namespace",
129+
},
130+
Spec: corev1.ServiceSpec{
131+
ClusterIP: "4.3.2.1",
132+
},
133+
},
134+
Expected: []clienttesting.Action{
135+
wildcard,
136+
clienttesting.NewPatchAction(gvr, "namespace", "name", types.StrategicMergePatchType, []byte(`{"spec":{"clusterIP":"4.3.2.1"}}`)),
137+
},
138+
},
139+
{
140+
Name: "spec modified",
141+
Old: &corev1.Service{
142+
ObjectMeta: metav1.ObjectMeta{
143+
Name: "name",
144+
Namespace: "namespace",
145+
},
146+
Spec: corev1.ServiceSpec{
147+
SessionAffinity: "None",
148+
},
149+
},
150+
New: &corev1.Service{
151+
ObjectMeta: metav1.ObjectMeta{
152+
Name: "name",
153+
Namespace: "namespace",
154+
},
155+
Spec: corev1.ServiceSpec{
156+
SessionAffinity: "ClientIP",
157+
},
158+
},
159+
Expected: []clienttesting.Action{
160+
wildcard,
161+
clienttesting.NewPatchAction(gvr, "namespace", "name", types.StrategicMergePatchType, []byte(`{"spec":{"sessionAffinity":"ClientIP"}}`)),
162+
},
163+
},
164+
} {
165+
t.Run(tc.Name, func(t *testing.T) {
166+
require := require.New(t)
167+
168+
kube := fake.NewSimpleClientset(tc.Old)
169+
c := &Client{
170+
Interface: kube,
171+
}
172+
173+
_, err := c.UpdateService(tc.New)
174+
require.NoError(err)
175+
176+
actual := kube.Actions()
177+
require.Len(actual, len(tc.Expected))
178+
179+
for i, action := range kube.Actions() {
180+
if tc.Expected[i] == wildcard {
181+
continue
182+
}
183+
require.Equal(tc.Expected[i], action)
184+
}
185+
})
186+
}
187+
}

‎test/e2e/installplan_e2e_test.go

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package e2e
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/json"
67
"errors"
@@ -9,14 +10,11 @@ import (
910
"sync"
1011
"time"
1112

12-
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog"
13-
14-
"github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx"
15-
1613
"github.com/blang/semver"
1714
. "github.com/onsi/ginkgo"
1815
"github.com/onsi/ginkgo/extensions/table"
1916
. "github.com/onsi/gomega"
17+
"github.com/onsi/gomega/types"
2018
"github.com/stretchr/testify/assert"
2119
"github.com/stretchr/testify/require"
2220
appsv1 "k8s.io/api/apps/v1"
@@ -27,25 +25,113 @@ import (
2725
k8serrors "k8s.io/apimachinery/pkg/api/errors"
2826
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2927
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
28+
"k8s.io/apimachinery/pkg/runtime"
29+
k8sjson "k8s.io/apimachinery/pkg/runtime/serializer/json"
3030
"k8s.io/apimachinery/pkg/util/wait"
3131
"k8s.io/apimachinery/pkg/watch"
3232
"k8s.io/client-go/util/retry"
33+
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
3334

3435
opver "github.com/operator-framework/api/pkg/lib/version"
3536
operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
3637
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
3738
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
39+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog"
3840
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
3941
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubernetes/pkg/apis/rbac"
4042
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
4143
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
44+
"github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx"
4245
)
4346

4447
var _ = Describe("Install Plan", func() {
4548
AfterEach(func() {
4649
TearDown(testNamespace)
4750
})
4851

52+
When("a ClusterIP service exists", func() {
53+
var (
54+
service *corev1.Service
55+
)
56+
57+
BeforeEach(func() {
58+
service = &corev1.Service{
59+
TypeMeta: metav1.TypeMeta{
60+
Kind: "Service",
61+
APIVersion: "v1",
62+
},
63+
ObjectMeta: metav1.ObjectMeta{
64+
Namespace: testNamespace,
65+
Name: "test-service",
66+
},
67+
Spec: corev1.ServiceSpec{
68+
Type: corev1.ServiceTypeClusterIP,
69+
Ports: []corev1.ServicePort{
70+
{
71+
Port: 12345,
72+
},
73+
},
74+
},
75+
}
76+
77+
Expect(ctx.Ctx().Client().Create(context.Background(), service.DeepCopy())).To(Succeed())
78+
})
79+
80+
AfterEach(func() {
81+
Expect(ctx.Ctx().Client().Delete(context.Background(), service)).To(Succeed())
82+
})
83+
84+
It("it can be updated by an InstallPlan step", func() {
85+
scheme := runtime.NewScheme()
86+
Expect(corev1.AddToScheme(scheme)).To(Succeed())
87+
var manifest bytes.Buffer
88+
Expect(k8sjson.NewSerializer(k8sjson.DefaultMetaFactory, scheme, scheme, false).Encode(service, &manifest)).To(Succeed())
89+
90+
plan := &operatorsv1alpha1.InstallPlan{
91+
ObjectMeta: metav1.ObjectMeta{
92+
Namespace: testNamespace,
93+
Name: "test-plan",
94+
},
95+
Spec: operatorsv1alpha1.InstallPlanSpec{
96+
Approval: operatorsv1alpha1.ApprovalAutomatic,
97+
Approved: true,
98+
ClusterServiceVersionNames: []string{},
99+
},
100+
Status: operatorsv1alpha1.InstallPlanStatus{
101+
Phase: operatorsv1alpha1.InstallPlanPhaseInstalling,
102+
CatalogSources: []string{},
103+
Plan: []*operatorsv1alpha1.Step{
104+
{
105+
Status: operatorsv1alpha1.StepStatusUnknown,
106+
Resource: operatorsv1alpha1.StepResource{
107+
Name: service.Name,
108+
Version: "v1",
109+
Kind: "Service",
110+
Manifest: manifest.String(),
111+
},
112+
},
113+
},
114+
},
115+
}
116+
117+
Expect(ctx.Ctx().Client().Create(context.Background(), plan)).To(Succeed())
118+
Expect(ctx.Ctx().Client().Status().Update(context.Background(), plan)).To(Succeed())
119+
120+
key, err := runtimeclient.ObjectKeyFromObject(plan)
121+
Expect(err).ToNot(HaveOccurred())
122+
123+
HavePhase := func(goal operatorsv1alpha1.InstallPlanPhase) types.GomegaMatcher {
124+
return WithTransform(func(plan *operatorsv1alpha1.InstallPlan) operatorsv1alpha1.InstallPlanPhase {
125+
return plan.Status.Phase
126+
}, Equal(goal))
127+
}
128+
129+
Eventually(func() (*operatorsv1alpha1.InstallPlan, error) {
130+
return plan, ctx.Ctx().Client().Get(context.Background(), key, plan)
131+
}).Should(HavePhase(operatorsv1alpha1.InstallPlanPhaseComplete))
132+
})
133+
})
134+
49135
It("with CSVs across multiple catalog sources", func() {
50136

51137
log := func(s string) {

0 commit comments

Comments
 (0)