Skip to content

Commit 8f8b25b

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #16166 from mfojtik/apps-use-patch
Automatic merge from submit-queue apps: use patch when pausing dc on delete Ignore the first 2 commits, they are part of #16149 Don't try to emulate PATCH behavior with retrying updates on conflicts (of course patch can also conflict, but chances are low). Also we use PATCH for pausing in `oc rollout pause` already, so this just unify this behavior. Ref: #16149 (comment)
2 parents 3d2f5f8 + c008a14 commit 8f8b25b

File tree

2 files changed

+24
-48
lines changed

2 files changed

+24
-48
lines changed

‎pkg/apps/cmd/delete.go

+3-28
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,14 @@ import (
66
"github.com/golang/glog"
77
kerrors "k8s.io/apimachinery/pkg/api/errors"
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/types"
910
"k8s.io/apimachinery/pkg/util/wait"
1011
kapi "k8s.io/kubernetes/pkg/api"
1112
kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
12-
"k8s.io/kubernetes/pkg/client/retry"
1313
"k8s.io/kubernetes/pkg/kubectl"
14-
kutil "k8s.io/kubernetes/pkg/util"
1514

1615
deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
1716
appsclient "github.com/openshift/origin/pkg/apps/generated/internalclientset"
18-
appsinternal "github.com/openshift/origin/pkg/apps/generated/internalclientset/typed/apps/internalversion"
1917
"github.com/openshift/origin/pkg/apps/util"
2018
)
2119

@@ -31,34 +29,11 @@ type DeploymentConfigReaper struct {
3129
pollInterval, timeout time.Duration
3230
}
3331

34-
type updateConfigFunc func(d *deployapi.DeploymentConfig)
35-
36-
// updateConfigWithRetries will try to update a deployment config and ignore any update conflicts.
37-
func updateConfigWithRetries(dn appsinternal.DeploymentConfigsGetter, namespace, name string, applyUpdate updateConfigFunc) (*deployapi.DeploymentConfig, error) {
38-
var config *deployapi.DeploymentConfig
39-
40-
resultErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
41-
var err error
42-
config, err = dn.DeploymentConfigs(namespace).Get(name, metav1.GetOptions{})
43-
if err != nil {
44-
return err
45-
}
46-
// Apply the update, then attempt to push it to the apiserver.
47-
applyUpdate(config)
48-
config, err = dn.DeploymentConfigs(namespace).Update(config)
49-
return err
50-
})
51-
return config, resultErr
52-
}
53-
5432
// pause marks the deployment configuration as paused to avoid triggering new
5533
// deployments.
5634
func (reaper *DeploymentConfigReaper) pause(namespace, name string) (*deployapi.DeploymentConfig, error) {
57-
return updateConfigWithRetries(reaper.appsClient.Apps(), namespace, name, func(d *deployapi.DeploymentConfig) {
58-
d.Spec.RevisionHistoryLimit = kutil.Int32Ptr(0)
59-
d.Spec.Replicas = 0
60-
d.Spec.Paused = true
61-
})
35+
patchBytes := []byte(`{"spec":{"paused":true,"replicas":0,"revisionHistoryLimit":0}}`)
36+
return reaper.appsClient.Apps().DeploymentConfigs(namespace).Patch(name, types.StrategicMergePatchType, patchBytes)
6237
}
6338

6439
// Stop scales a replication controller via its deployment configuration down to

‎pkg/apps/cmd/delete_test.go

+21-20
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import (
55
"testing"
66
"time"
77

8+
kapierrors "k8s.io/apimachinery/pkg/api/errors"
89
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910
"k8s.io/apimachinery/pkg/labels"
11+
"k8s.io/apimachinery/pkg/runtime"
1012
"k8s.io/apimachinery/pkg/runtime/schema"
1113
"k8s.io/apimachinery/pkg/util/diff"
1214
clientgotesting "k8s.io/client-go/testing"
@@ -40,10 +42,7 @@ func TestStop(t *testing.T) {
4042
replicationControllerKind = schema.GroupVersionKind{Kind: "ReplicationController"}
4143
)
4244

43-
pause := func(d *deployapi.DeploymentConfig) *deployapi.DeploymentConfig {
44-
d.Spec.Paused = true
45-
return d
46-
}
45+
pauseBytes := []byte(`{"spec":{"paused":true,"replicas":0,"revisionHistoryLimit":0}}`)
4746

4847
fakeDC := map[string]*deployapi.DeploymentConfig{
4948
"simple-stop": deploytest.OkDeploymentConfig(1),
@@ -54,6 +53,14 @@ func TestStop(t *testing.T) {
5453
"legacy-no-deployments": deploytest.OkDeploymentConfig(5),
5554
}
5655

56+
emptyClientset := func() *appsfake.Clientset {
57+
result := &appsfake.Clientset{}
58+
result.AddReactor("patch", "deploymentconfigs", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
59+
return true, nil, kapierrors.NewNotFound(schema.GroupResource{Group: "apps.openshift.io", Resource: "deploymentconfig"}, "config")
60+
})
61+
return result
62+
}
63+
5764
tests := []struct {
5865
testName string
5966
namespace string
@@ -71,8 +78,7 @@ func TestStop(t *testing.T) {
7178
oc: appsfake.NewSimpleClientset(fakeDC["simple-stop"]),
7279
kc: fake.NewSimpleClientset(mkdeploymentlist(1)),
7380
expected: []clientgotesting.Action{
74-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
75-
clientgotesting.NewUpdateAction(deploymentConfigsResource, "default", pause(fakeDC["simple-stop"])),
81+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
7682
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
7783
clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"),
7884
},
@@ -94,8 +100,7 @@ func TestStop(t *testing.T) {
94100
oc: appsfake.NewSimpleClientset(fakeDC["legacy-simple-stop"]),
95101
kc: fake.NewSimpleClientset(mkdeploymentlist(1)),
96102
expected: []clientgotesting.Action{
97-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
98-
clientgotesting.NewUpdateAction(deploymentConfigsResource, "default", nil),
103+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
99104
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
100105
clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"),
101106
},
@@ -117,8 +122,7 @@ func TestStop(t *testing.T) {
117122
oc: appsfake.NewSimpleClientset(fakeDC["multi-stop"]),
118123
kc: fake.NewSimpleClientset(mkdeploymentlist(1, 2, 3, 4, 5)),
119124
expected: []clientgotesting.Action{
120-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
121-
clientgotesting.NewUpdateAction(deploymentConfigsResource, "default", pause(fakeDC["multi-stop"])),
125+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
122126
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
123127
clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"),
124128
},
@@ -164,8 +168,7 @@ func TestStop(t *testing.T) {
164168
oc: appsfake.NewSimpleClientset(fakeDC["legacy-multi-stop"]),
165169
kc: fake.NewSimpleClientset(mkdeploymentlist(1, 2, 3, 4, 5)),
166170
expected: []clientgotesting.Action{
167-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
168-
clientgotesting.NewUpdateAction(deploymentConfigsResource, "default", nil),
171+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
169172
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
170173
clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"),
171174
},
@@ -208,10 +211,10 @@ func TestStop(t *testing.T) {
208211
testName: "no config, some deployments",
209212
namespace: "default",
210213
name: "config",
211-
oc: appsfake.NewSimpleClientset(),
214+
oc: emptyClientset(),
212215
kc: fake.NewSimpleClientset(mkdeploymentlist(1)),
213216
expected: []clientgotesting.Action{
214-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
217+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
215218
},
216219
kexpected: []clientgotesting.Action{
217220
clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}),
@@ -228,10 +231,10 @@ func TestStop(t *testing.T) {
228231
testName: "no config, no deployments",
229232
namespace: "default",
230233
name: "config",
231-
oc: appsfake.NewSimpleClientset(),
234+
oc: emptyClientset(),
232235
kc: fake.NewSimpleClientset(&kapi.ReplicationControllerList{}),
233236
expected: []clientgotesting.Action{
234-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
237+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
235238
},
236239
kexpected: []clientgotesting.Action{
237240
clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{}),
@@ -245,8 +248,7 @@ func TestStop(t *testing.T) {
245248
oc: appsfake.NewSimpleClientset(fakeDC["no-deployments"]),
246249
kc: fake.NewSimpleClientset(&kapi.ReplicationControllerList{}),
247250
expected: []clientgotesting.Action{
248-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
249-
clientgotesting.NewUpdateAction(deploymentConfigsResource, "default", pause(fakeDC["no-deployments"])),
251+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
250252
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
251253
clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"),
252254
},
@@ -262,8 +264,7 @@ func TestStop(t *testing.T) {
262264
oc: appsfake.NewSimpleClientset(fakeDC["legacy-no-deployments"]),
263265
kc: fake.NewSimpleClientset(&kapi.ReplicationControllerList{}),
264266
expected: []clientgotesting.Action{
265-
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
266-
clientgotesting.NewUpdateAction(deploymentConfigsResource, "default", nil),
267+
clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes),
267268
clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"),
268269
clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"),
269270
},

0 commit comments

Comments
 (0)