Skip to content

Commit 9e17245

Browse files
author
OpenShift Bot
authored
Merge pull request #9893 from mfojtik/deploy-pause-reaper
Merged by openshift-bot
2 parents 6a5f284 + 51c2eac commit 9e17245

File tree

3 files changed

+221
-22
lines changed

3 files changed

+221
-22
lines changed

‎pkg/deploy/cmd/reaper.go

Lines changed: 86 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ import (
88
kerrors "k8s.io/kubernetes/pkg/api/errors"
99
kclient "k8s.io/kubernetes/pkg/client/unversioned"
1010
"k8s.io/kubernetes/pkg/kubectl"
11+
kutil "k8s.io/kubernetes/pkg/util"
12+
"k8s.io/kubernetes/pkg/util/wait"
1113

1214
"github.com/openshift/origin/pkg/client"
15+
deployapi "github.com/openshift/origin/pkg/deploy/api"
1316
"github.com/openshift/origin/pkg/deploy/util"
1417
)
1518

@@ -25,26 +28,60 @@ type DeploymentConfigReaper struct {
2528
pollInterval, timeout time.Duration
2629
}
2730

31+
// pause marks the deployment configuration as paused to avoid triggering new
32+
// deployments.
33+
func (reaper *DeploymentConfigReaper) pause(namespace, name string) (*deployapi.DeploymentConfig, error) {
34+
return reaper.updateDeploymentWithRetries(namespace, name, func(d *deployapi.DeploymentConfig) {
35+
d.Spec.RevisionHistoryLimit = kutil.Int32Ptr(0)
36+
d.Spec.Replicas = 0
37+
d.Spec.Paused = true
38+
})
39+
}
40+
2841
// Stop scales a replication controller via its deployment configuration down to
2942
// zero replicas, waits for all of them to get deleted and then deletes both the
3043
// replication controller and its deployment configuration.
3144
func (reaper *DeploymentConfigReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *kapi.DeleteOptions) error {
32-
// If the config is already deleted, it may still have associated
33-
// deployments which didn't get cleaned up during prior calls to Stop. If
34-
// the config can't be found, still make an attempt to clean up the
35-
// deployments.
36-
//
37-
// It's important to delete the config first to avoid an undesirable side
38-
// effect which can cause the deployment to be re-triggered upon the
39-
// config's deletion. See https://github.com/openshift/origin/issues/2721
40-
// for more details.
41-
err := reaper.oc.DeploymentConfigs(namespace).Delete(name)
45+
// Pause the deployment configuration to prevent the new deployments from
46+
// being triggered.
47+
config, err := reaper.pause(namespace, name)
4248
configNotFound := kerrors.IsNotFound(err)
4349
if err != nil && !configNotFound {
4450
return err
4551
}
4652

47-
// Clean up deployments related to the config.
53+
var (
54+
isPaused bool
55+
legacy bool
56+
)
57+
// Determine if the deployment config controller noticed the pause.
58+
if !configNotFound {
59+
if err := wait.Poll(1*time.Second, 1*time.Minute, func() (bool, error) {
60+
dc, err := reaper.oc.DeploymentConfigs(namespace).Get(name)
61+
if err != nil {
62+
return false, err
63+
}
64+
isPaused = dc.Spec.Paused
65+
return dc.Status.ObservedGeneration >= config.Generation, nil
66+
}); err != nil {
67+
return err
68+
}
69+
70+
// If we failed to pause the deployment config, it means we are talking to
71+
// old API that does not support pausing. In that case, we delete the
72+
// deployment config to stay backward compatible.
73+
if !isPaused {
74+
if err := reaper.oc.DeploymentConfigs(namespace).Delete(name); err != nil {
75+
return err
76+
}
77+
// Setting this to true avoid deleting the config at the end.
78+
legacy = true
79+
}
80+
}
81+
82+
// Clean up deployments related to the config. Even if the deployment
83+
// configuration has been deleted, we want to sweep the existing replication
84+
// controllers and clean them up.
4885
options := kapi.ListOptions{LabelSelector: util.ConfigSelector(name)}
4986
rcList, err := reaper.kc.ReplicationControllers(namespace).List(options)
5087
if err != nil {
@@ -61,10 +98,11 @@ func (reaper *DeploymentConfigReaper) Stop(namespace, name string, timeout time.
6198
if configNotFound && len(deployments) == 0 {
6299
return kerrors.NewNotFound(kapi.Resource("deploymentconfig"), name)
63100
}
101+
64102
for _, rc := range deployments {
65103
if err = rcReaper.Stop(rc.Namespace, rc.Name, timeout, gracePeriod); err != nil {
66104
// Better not error out here...
67-
glog.Infof("Cannot delete ReplicationController %s/%s: %v", rc.Namespace, rc.Name, err)
105+
glog.Infof("Cannot delete ReplicationController %s/%s for deployment config %s/%s: %v", rc.Namespace, rc.Name, namespace, name, err)
68106
}
69107

70108
// Only remove deployer pods when the deployment was failed. For completed
@@ -83,10 +121,44 @@ func (reaper *DeploymentConfigReaper) Stop(namespace, name string, timeout time.
83121
err := reaper.kc.Pods(pod.Namespace).Delete(pod.Name, gracePeriod)
84122
if err != nil {
85123
// Better not error out here...
86-
glog.Infof("Cannot delete Pod %s/%s: %v", pod.Namespace, pod.Name, err)
124+
glog.Infof("Cannot delete lifecycle Pod %s/%s for deployment config %s/%s: %v", pod.Namespace, pod.Name, namespace, name, err)
87125
}
88126
}
89127
}
90128

91-
return nil
129+
// Nothing to delete or we already deleted the deployment config because we
130+
// failed to pause.
131+
if configNotFound || legacy {
132+
return nil
133+
}
134+
135+
return reaper.oc.DeploymentConfigs(namespace).Delete(name)
136+
}
137+
138+
type updateConfigFunc func(d *deployapi.DeploymentConfig)
139+
140+
func (reaper *DeploymentConfigReaper) updateDeploymentWithRetries(namespace, name string, applyUpdate updateConfigFunc) (*deployapi.DeploymentConfig, error) {
141+
var (
142+
config *deployapi.DeploymentConfig
143+
err error
144+
)
145+
deploymentConfigs := reaper.oc.DeploymentConfigs(namespace)
146+
resultErr := wait.Poll(10*time.Millisecond, 1*time.Minute, func() (bool, error) {
147+
config, err = deploymentConfigs.Get(name)
148+
if err != nil {
149+
return false, err
150+
}
151+
// Apply the update, then attempt to push it to the apiserver.
152+
applyUpdate(config)
153+
config, err = deploymentConfigs.Update(config)
154+
if err != nil {
155+
// Retry only on update conflict
156+
if kerrors.IsConflict(err) {
157+
return false, nil
158+
}
159+
return false, err
160+
}
161+
return true, nil
162+
})
163+
return config, resultErr
92164
}

‎pkg/deploy/cmd/reaper_test.go

Lines changed: 134 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,20 @@ func TestStop(t *testing.T) {
3535
return &(kerrors.NewNotFound(kapi.Resource("DeploymentConfig"), "config").ErrStatus)
3636
}
3737

38+
pause := func(d *deployapi.DeploymentConfig) *deployapi.DeploymentConfig {
39+
d.Spec.Paused = true
40+
return d
41+
}
42+
43+
fakeDC := map[string]*deployapi.DeploymentConfig{
44+
"simple-stop": deploytest.OkDeploymentConfig(1),
45+
"legacy-simple-stop": deploytest.OkDeploymentConfig(1),
46+
"multi-stop": deploytest.OkDeploymentConfig(5),
47+
"legacy-multi-stop": deploytest.OkDeploymentConfig(5),
48+
"no-deployments": deploytest.OkDeploymentConfig(5),
49+
"legacy-no-deployments": deploytest.OkDeploymentConfig(5),
50+
}
51+
3852
tests := []struct {
3953
testName string
4054
namespace string
@@ -49,9 +63,36 @@ func TestStop(t *testing.T) {
4963
testName: "simple stop",
5064
namespace: "default",
5165
name: "config",
52-
oc: testclient.NewSimpleFake(deploytest.OkDeploymentConfig(1)),
66+
oc: testclient.NewSimpleFake(fakeDC["simple-stop"]),
67+
kc: ktestclient.NewSimpleFake(mkdeploymentlist(1)),
68+
expected: []ktestclient.Action{
69+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
70+
ktestclient.NewUpdateAction("deploymentconfigs", "default", pause(fakeDC["simple-stop"])),
71+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
72+
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
73+
},
74+
kexpected: []ktestclient.Action{
75+
ktestclient.NewListAction("replicationcontrollers", "default", kapi.ListOptions{}),
76+
ktestclient.NewGetAction("replicationcontrollers", "", "config-1"),
77+
ktestclient.NewListAction("replicationcontrollers", "", kapi.ListOptions{}),
78+
ktestclient.NewGetAction("replicationcontrollers", "", "config-1"),
79+
ktestclient.NewUpdateAction("replicationcontrollers", "", nil),
80+
ktestclient.NewGetAction("replicationcontrollers", "", "config-1"),
81+
ktestclient.NewGetAction("replicationcontrollers", "", "config-1"),
82+
ktestclient.NewDeleteAction("replicationcontrollers", "", "config-1"),
83+
},
84+
err: false,
85+
},
86+
{
87+
testName: "legacy simple stop",
88+
namespace: "default",
89+
name: "config",
90+
oc: testclient.NewSimpleFake(fakeDC["legacy-simple-stop"]),
5391
kc: ktestclient.NewSimpleFake(mkdeploymentlist(1)),
5492
expected: []ktestclient.Action{
93+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
94+
ktestclient.NewUpdateAction("deploymentconfigs", "default", nil),
95+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
5596
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
5697
},
5798
kexpected: []ktestclient.Action{
@@ -70,9 +111,64 @@ func TestStop(t *testing.T) {
70111
testName: "stop multiple controllers",
71112
namespace: "default",
72113
name: "config",
73-
oc: testclient.NewSimpleFake(deploytest.OkDeploymentConfig(5)),
114+
oc: testclient.NewSimpleFake(fakeDC["multi-stop"]),
115+
kc: ktestclient.NewSimpleFake(mkdeploymentlist(1, 2, 3, 4, 5)),
116+
expected: []ktestclient.Action{
117+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
118+
ktestclient.NewUpdateAction("deploymentconfigs", "default", pause(fakeDC["multi-stop"])),
119+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
120+
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
121+
},
122+
kexpected: []ktestclient.Action{
123+
ktestclient.NewListAction("replicationcontrollers", "default", kapi.ListOptions{}),
124+
ktestclient.NewGetAction("replicationcontrollers", "", "config-1"),
125+
ktestclient.NewListAction("replicationcontrollers", "", kapi.ListOptions{}),
126+
ktestclient.NewGetAction("replicationcontrollers", "", "config-1"),
127+
ktestclient.NewUpdateAction("replicationcontrollers", "", nil),
128+
ktestclient.NewGetAction("replicationcontrollers", "", "config-1"),
129+
ktestclient.NewGetAction("replicationcontrollers", "", "config-4"),
130+
ktestclient.NewDeleteAction("replicationcontrollers", "", "config-1"),
131+
ktestclient.NewGetAction("replicationcontrollers", "", "config-2"),
132+
ktestclient.NewListAction("replicationcontrollers", "", kapi.ListOptions{}),
133+
ktestclient.NewGetAction("replicationcontrollers", "", "config-2"),
134+
ktestclient.NewUpdateAction("replicationcontrollers", "", nil),
135+
ktestclient.NewGetAction("replicationcontrollers", "", "config-2"),
136+
ktestclient.NewGetAction("replicationcontrollers", "", "config-5"),
137+
ktestclient.NewDeleteAction("replicationcontrollers", "", "config-2"),
138+
ktestclient.NewGetAction("replicationcontrollers", "", "config-3"),
139+
ktestclient.NewListAction("replicationcontrollers", "", kapi.ListOptions{}),
140+
ktestclient.NewGetAction("replicationcontrollers", "", "config-3"),
141+
ktestclient.NewUpdateAction("replicationcontrollers", "", nil),
142+
ktestclient.NewGetAction("replicationcontrollers", "", "config-3"),
143+
ktestclient.NewGetAction("replicationcontrollers", "", "config-5"),
144+
ktestclient.NewDeleteAction("replicationcontrollers", "", "config-3"),
145+
ktestclient.NewGetAction("replicationcontrollers", "", "config-4"),
146+
ktestclient.NewListAction("replicationcontrollers", "", kapi.ListOptions{}),
147+
ktestclient.NewGetAction("replicationcontrollers", "", "config-4"),
148+
ktestclient.NewUpdateAction("replicationcontrollers", "", nil),
149+
ktestclient.NewGetAction("replicationcontrollers", "", "config-4"),
150+
ktestclient.NewGetAction("replicationcontrollers", "", "config-5"),
151+
ktestclient.NewDeleteAction("replicationcontrollers", "", "config-4"),
152+
ktestclient.NewGetAction("replicationcontrollers", "", "config-5"),
153+
ktestclient.NewListAction("replicationcontrollers", "", kapi.ListOptions{}),
154+
ktestclient.NewGetAction("replicationcontrollers", "", "config-5"),
155+
ktestclient.NewUpdateAction("replicationcontrollers", "", nil),
156+
ktestclient.NewGetAction("replicationcontrollers", "", "config-5"),
157+
ktestclient.NewGetAction("replicationcontrollers", "", "config-5"),
158+
ktestclient.NewDeleteAction("replicationcontrollers", "", "config-5"),
159+
},
160+
err: false,
161+
},
162+
{
163+
testName: "legacy stop multiple controllers",
164+
namespace: "default",
165+
name: "config",
166+
oc: testclient.NewSimpleFake(fakeDC["legacy-multi-stop"]),
74167
kc: ktestclient.NewSimpleFake(mkdeploymentlist(1, 2, 3, 4, 5)),
75168
expected: []ktestclient.Action{
169+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
170+
ktestclient.NewUpdateAction("deploymentconfigs", "default", nil),
171+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
76172
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
77173
},
78174
kexpected: []ktestclient.Action{
@@ -122,7 +218,7 @@ func TestStop(t *testing.T) {
122218
oc: testclient.NewSimpleFake(notfound()),
123219
kc: ktestclient.NewSimpleFake(mkdeploymentlist(1)),
124220
expected: []ktestclient.Action{
125-
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
221+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
126222
},
127223
kexpected: []ktestclient.Action{
128224
ktestclient.NewListAction("replicationcontrollers", "default", kapi.ListOptions{}),
@@ -143,7 +239,7 @@ func TestStop(t *testing.T) {
143239
oc: testclient.NewSimpleFake(notfound()),
144240
kc: ktestclient.NewSimpleFake(&kapi.ReplicationControllerList{}),
145241
expected: []ktestclient.Action{
146-
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
242+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
147243
},
148244
kexpected: []ktestclient.Action{
149245
ktestclient.NewListAction("replicationcontrollers", "default", kapi.ListOptions{}),
@@ -154,9 +250,29 @@ func TestStop(t *testing.T) {
154250
testName: "config, no deployments",
155251
namespace: "default",
156252
name: "config",
157-
oc: testclient.NewSimpleFake(deploytest.OkDeploymentConfig(5)),
253+
oc: testclient.NewSimpleFake(fakeDC["no-deployments"]),
254+
kc: ktestclient.NewSimpleFake(&kapi.ReplicationControllerList{}),
255+
expected: []ktestclient.Action{
256+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
257+
ktestclient.NewUpdateAction("deploymentconfigs", "default", pause(fakeDC["no-deployments"])),
258+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
259+
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
260+
},
261+
kexpected: []ktestclient.Action{
262+
ktestclient.NewListAction("replicationcontrollers", "default", kapi.ListOptions{}),
263+
},
264+
err: false,
265+
},
266+
{
267+
testName: "legacy config, no deployments",
268+
namespace: "default",
269+
name: "config",
270+
oc: testclient.NewSimpleFake(fakeDC["legacy-no-deployments"]),
158271
kc: ktestclient.NewSimpleFake(&kapi.ReplicationControllerList{}),
159272
expected: []ktestclient.Action{
273+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
274+
ktestclient.NewUpdateAction("deploymentconfigs", "default", nil),
275+
ktestclient.NewGetAction("deploymentconfigs", "default", "config"),
160276
ktestclient.NewDeleteAction("deploymentconfigs", "default", "config"),
161277
},
162278
kexpected: []ktestclient.Action{
@@ -180,8 +296,19 @@ func TestStop(t *testing.T) {
180296
t.Fatalf("%s: unexpected actions: %v, expected %v", test.testName, test.oc.Actions(), test.expected)
181297
}
182298
for j, actualAction := range test.oc.Actions() {
183-
if !reflect.DeepEqual(actualAction, test.expected[j]) {
184-
t.Errorf("%s: unexpected action: %s, expected %s", test.testName, actualAction, test.expected[j])
299+
e, a := test.expected[j], actualAction
300+
switch a.(type) {
301+
case ktestclient.UpdateAction:
302+
if e.GetVerb() != a.GetVerb() ||
303+
e.GetNamespace() != a.GetNamespace() ||
304+
e.GetResource() != a.GetResource() ||
305+
e.GetSubresource() != a.GetSubresource() {
306+
t.Errorf("%s: unexpected action[%d]: %s, expected %s", test.testName, j, a, e)
307+
}
308+
default:
309+
if !reflect.DeepEqual(actualAction, test.expected[j]) {
310+
t.Errorf("%s: unexpected action: got:\n%#+v\nexpected:\n%#+v", test.testName, actualAction, test.expected[j])
311+
}
185312
}
186313
}
187314
if len(test.kc.Actions()) != len(test.kexpected) {

‎test/extended/deployments/deployments.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ var _ = g.Describe("deploymentconfigs", func() {
483483
})
484484

485485
g.Describe("reaper", func() {
486-
g.It("should delete all failed deployer pods and hook pods", func() {
486+
g.It("should delete all failed deployer pods and hook pods [Conformance]", func() {
487487
resource, name, err := createFixture(oc, brokenDeploymentFixture)
488488
o.Expect(err).NotTo(o.HaveOccurred())
489489

0 commit comments

Comments
 (0)