Skip to content

Commit ffc2947

Browse files
authored
Merge pull request kubernetes#80772 from seans3/move-scale-updater-staging
Refactor pkg/kubectl/{scale.go|rollingupdater.go} for move to staging
2 parents 4174516 + c011df2 commit ffc2947

File tree

16 files changed

+171
-106
lines changed

16 files changed

+171
-106
lines changed

hack/.golint_failures

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ pkg/credentialprovider/gcp
106106
pkg/features
107107
pkg/kubeapiserver
108108
pkg/kubeapiserver/options
109-
pkg/kubectl
110109
pkg/kubectl/cmd/annotate
111110
pkg/kubectl/cmd/apply
112111
pkg/kubectl/cmd/attach
@@ -536,6 +535,7 @@ staging/src/k8s.io/kubectl/pkg/generate
536535
staging/src/k8s.io/kubectl/pkg/generate/versioned
537536
staging/src/k8s.io/kubectl/pkg/metricsutil
538537
staging/src/k8s.io/kubectl/pkg/polymorphichelpers
538+
staging/src/k8s.io/kubectl/pkg/scale
539539
staging/src/k8s.io/kubectl/pkg/util/templates
540540
staging/src/k8s.io/kubelet/config/v1beta1
541541
staging/src/k8s.io/legacy-cloud-providers/vsphere

pkg/kubectl/BUILD

Lines changed: 3 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,70 +1,21 @@
1-
package(default_visibility = ["//visibility:public"])
2-
3-
load(
4-
"@io_bazel_rules_go//go:def.bzl",
5-
"go_library",
6-
"go_test",
7-
)
8-
9-
go_test(
10-
name = "go_default_test",
11-
srcs = [
12-
"rolling_updater_test.go",
13-
"scale_test.go",
14-
],
15-
embed = [":go_default_library"],
16-
deps = [
17-
"//staging/src/k8s.io/api/autoscaling/v1:go_default_library",
18-
"//staging/src/k8s.io/api/core/v1:go_default_library",
19-
"//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
20-
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
21-
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
22-
"//staging/src/k8s.io/apimachinery/pkg/apis/testapigroup/v1:go_default_library",
23-
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
24-
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
25-
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
26-
"//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
27-
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
28-
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
29-
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
30-
"//staging/src/k8s.io/client-go/rest:go_default_library",
31-
"//staging/src/k8s.io/client-go/rest/fake:go_default_library",
32-
"//staging/src/k8s.io/client-go/scale:go_default_library",
33-
"//staging/src/k8s.io/client-go/scale/fake:go_default_library",
34-
"//staging/src/k8s.io/client-go/testing:go_default_library",
35-
"//staging/src/k8s.io/kubectl/pkg/scheme:go_default_library",
36-
"//staging/src/k8s.io/kubectl/pkg/util:go_default_library",
37-
],
38-
)
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
392

403
go_library(
414
name = "go_default_library",
425
srcs = [
436
"conditions.go",
447
"doc.go",
45-
"rolling_updater.go",
46-
"scale.go",
478
],
489
importpath = "k8s.io/kubernetes/pkg/kubectl",
10+
visibility = ["//visibility:public"],
4911
deps = [
50-
"//staging/src/k8s.io/api/autoscaling/v1:go_default_library",
5112
"//staging/src/k8s.io/api/core/v1:go_default_library",
5213
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
5314
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
54-
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
55-
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
5615
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
57-
"//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
5816
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
5917
"//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
6018
"//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
61-
"//staging/src/k8s.io/client-go/scale:go_default_library",
62-
"//staging/src/k8s.io/client-go/util/retry:go_default_library",
63-
"//staging/src/k8s.io/kubectl/pkg/util:go_default_library",
64-
"//staging/src/k8s.io/kubectl/pkg/util/deployment:go_default_library",
65-
"//staging/src/k8s.io/kubectl/pkg/util/podutils:go_default_library",
66-
"//vendor/k8s.io/utils/integer:go_default_library",
67-
"//vendor/k8s.io/utils/pointer:go_default_library",
6819
],
6920
)
7021

@@ -83,4 +34,5 @@ filegroup(
8334
"//pkg/kubectl/explain:all-srcs",
8435
],
8536
tags = ["automanaged"],
37+
visibility = ["//visibility:public"],
8638
)

pkg/kubectl/cmd/rollingupdate/BUILD

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,72 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "go_default_library",
5-
srcs = ["rollingupdate.go"],
5+
srcs = [
6+
"rolling_updater.go",
7+
"rollingupdate.go",
8+
],
69
importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/rollingupdate",
710
visibility = ["//visibility:public"],
811
deps = [
9-
"//pkg/kubectl:go_default_library",
1012
"//staging/src/k8s.io/api/core/v1:go_default_library",
1113
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
1214
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
15+
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
16+
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
17+
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
1318
"//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
1419
"//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
20+
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
1521
"//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library",
1622
"//staging/src/k8s.io/cli-runtime/pkg/printers:go_default_library",
1723
"//staging/src/k8s.io/cli-runtime/pkg/resource:go_default_library",
1824
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
25+
"//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
1926
"//staging/src/k8s.io/client-go/scale:go_default_library",
27+
"//staging/src/k8s.io/client-go/util/retry:go_default_library",
2028
"//staging/src/k8s.io/kubectl/pkg/cmd/util:go_default_library",
29+
"//staging/src/k8s.io/kubectl/pkg/scale:go_default_library",
2130
"//staging/src/k8s.io/kubectl/pkg/scheme:go_default_library",
2231
"//staging/src/k8s.io/kubectl/pkg/util:go_default_library",
32+
"//staging/src/k8s.io/kubectl/pkg/util/deployment:go_default_library",
2333
"//staging/src/k8s.io/kubectl/pkg/util/i18n:go_default_library",
34+
"//staging/src/k8s.io/kubectl/pkg/util/podutils:go_default_library",
2435
"//staging/src/k8s.io/kubectl/pkg/util/templates:go_default_library",
2536
"//staging/src/k8s.io/kubectl/pkg/validation:go_default_library",
2637
"//vendor/github.com/spf13/cobra:go_default_library",
2738
"//vendor/k8s.io/klog:go_default_library",
39+
"//vendor/k8s.io/utils/integer:go_default_library",
40+
"//vendor/k8s.io/utils/pointer:go_default_library",
2841
],
2942
)
3043

3144
go_test(
3245
name = "go_default_test",
33-
srcs = ["rollingupdate_test.go"],
46+
srcs = [
47+
"rolling_updater_test.go",
48+
"rollingupdate_test.go",
49+
],
3450
embed = [":go_default_library"],
3551
deps = [
3652
"//pkg/kubectl/cmd/testing:go_default_library",
53+
"//staging/src/k8s.io/api/core/v1:go_default_library",
54+
"//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
55+
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
56+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
57+
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
58+
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
59+
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
60+
"//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
61+
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
3762
"//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library",
63+
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
64+
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
65+
"//staging/src/k8s.io/client-go/rest:go_default_library",
66+
"//staging/src/k8s.io/client-go/rest/fake:go_default_library",
67+
"//staging/src/k8s.io/client-go/testing:go_default_library",
68+
"//staging/src/k8s.io/kubectl/pkg/scale:go_default_library",
69+
"//staging/src/k8s.io/kubectl/pkg/scheme:go_default_library",
70+
"//staging/src/k8s.io/kubectl/pkg/util:go_default_library",
3871
],
3972
)
4073

pkg/kubectl/rolling_updater.go renamed to pkg/kubectl/cmd/rollingupdate/rolling_updater.go

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package kubectl
17+
package rollingupdate
1818

1919
import (
2020
"fmt"
@@ -34,6 +34,7 @@ import (
3434
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
3535
scaleclient "k8s.io/client-go/scale"
3636
"k8s.io/client-go/util/retry"
37+
"k8s.io/kubectl/pkg/scale"
3738
"k8s.io/kubectl/pkg/util"
3839
deploymentutil "k8s.io/kubectl/pkg/util/deployment"
3940
"k8s.io/kubectl/pkg/util/podutils"
@@ -128,7 +129,7 @@ type RollingUpdater struct {
128129
// Namespace for resources
129130
ns string
130131
// scaleAndWait scales a controller and returns its updated state.
131-
scaleAndWait func(rc *corev1.ReplicationController, retry *RetryParams, wait *RetryParams) (*corev1.ReplicationController, error)
132+
scaleAndWait func(rc *corev1.ReplicationController, retry *scale.RetryParams, wait *scale.RetryParams) (*corev1.ReplicationController, error)
132133
//getOrCreateTargetController gets and validates an existing controller or
133134
//makes a new one.
134135
getOrCreateTargetController func(controller *corev1.ReplicationController, sourceID string) (*corev1.ReplicationController, bool, error)
@@ -180,7 +181,7 @@ func NewRollingUpdater(namespace string, rcClient corev1client.ReplicationContro
180181
func (r *RollingUpdater) Update(config *RollingUpdaterConfig) error {
181182
out := config.Out
182183
oldRc := config.OldRc
183-
scaleRetryParams := NewRetryParams(config.Interval, config.Timeout)
184+
scaleRetryParams := scale.NewRetryParams(config.Interval, config.Timeout)
184185

185186
// Find an existing controller (for continuing an interrupted update) or
186187
// create a new one if necessary.
@@ -321,7 +322,7 @@ func (r *RollingUpdater) Update(config *RollingUpdaterConfig) error {
321322
// scaleUp scales up newRc to desired by whatever increment is possible given
322323
// the configured surge threshold. scaleUp will safely no-op as necessary when
323324
// it detects redundancy or other relevant conditions.
324-
func (r *RollingUpdater) scaleUp(newRc, oldRc *corev1.ReplicationController, desired, maxSurge, maxUnavailable int32, scaleRetryParams *RetryParams, config *RollingUpdaterConfig) (*corev1.ReplicationController, error) {
325+
func (r *RollingUpdater) scaleUp(newRc, oldRc *corev1.ReplicationController, desired, maxSurge, maxUnavailable int32, scaleRetryParams *scale.RetryParams, config *RollingUpdaterConfig) (*corev1.ReplicationController, error) {
325326
// If we're already at the desired, do nothing.
326327
if valOrZero(newRc.Spec.Replicas) == desired {
327328
return newRc, nil
@@ -398,7 +399,10 @@ func (r *RollingUpdater) scaleDown(newRc, oldRc *corev1.ReplicationController, d
398399
}
399400
// Perform the scale-down.
400401
fmt.Fprintf(config.Out, "Scaling %s down to %d\n", oldRc.Name, valOrZero(oldRc.Spec.Replicas))
401-
retryWait := &RetryParams{config.Interval, config.Timeout}
402+
retryWait := &scale.RetryParams{
403+
Interval: config.Interval,
404+
Timeout: config.Timeout,
405+
}
402406
scaledRc, err := r.scaleAndWait(oldRc, retryWait, retryWait)
403407
if err != nil {
404408
return nil, err
@@ -407,9 +411,9 @@ func (r *RollingUpdater) scaleDown(newRc, oldRc *corev1.ReplicationController, d
407411
}
408412

409413
// scalerScaleAndWait scales a controller using a Scaler and a real client.
410-
func (r *RollingUpdater) scaleAndWaitWithScaler(rc *corev1.ReplicationController, retry *RetryParams, wait *RetryParams) (*corev1.ReplicationController, error) {
411-
scaler := NewScaler(r.scaleClient)
412-
if err := scaler.Scale(rc.Namespace, rc.Name, uint(valOrZero(rc.Spec.Replicas)), &ScalePrecondition{-1, ""}, retry, wait, schema.GroupResource{Resource: "replicationcontrollers"}); err != nil {
414+
func (r *RollingUpdater) scaleAndWaitWithScaler(rc *corev1.ReplicationController, retry *scale.RetryParams, wait *scale.RetryParams) (*corev1.ReplicationController, error) {
415+
scaler := scale.NewScaler(r.scaleClient)
416+
if err := scaler.Scale(rc.Namespace, rc.Name, uint(valOrZero(rc.Spec.Replicas)), &scale.ScalePrecondition{Size: -1, ResourceVersion: ""}, retry, wait, schema.GroupResource{Resource: "replicationcontrollers"}); err != nil {
413417
return nil, err
414418
}
415419
return r.rcClient.ReplicationControllers(rc.Namespace).Get(rc.Name, metav1.GetOptions{})
@@ -520,7 +524,7 @@ func (r *RollingUpdater) cleanupWithClients(oldRc, newRc *corev1.ReplicationCont
520524
return err
521525
}
522526

523-
if err = wait.Poll(config.Interval, config.Timeout, ControllerHasDesiredReplicas(r.rcClient, newRc)); err != nil {
527+
if err = wait.Poll(config.Interval, config.Timeout, controllerHasDesiredReplicas(r.rcClient, newRc)); err != nil {
524528
return err
525529
}
526530
newRc, err = r.rcClient.ReplicationControllers(r.ns).Get(newRc.Name, metav1.GetOptions{})
@@ -838,3 +842,24 @@ func FindSourceController(r corev1client.ReplicationControllersGetter, namespace
838842
}
839843
return nil, fmt.Errorf("couldn't find a replication controller with source id == %s/%s", namespace, name)
840844
}
845+
846+
// controllerHasDesiredReplicas returns a condition that will be true if and only if
847+
// the desired replica count for a controller's ReplicaSelector equals the Replicas count.
848+
func controllerHasDesiredReplicas(rcClient corev1client.ReplicationControllersGetter, controller *corev1.ReplicationController) wait.ConditionFunc {
849+
850+
// If we're given a controller where the status lags the spec, it either means that the controller is stale,
851+
// or that the rc manager hasn't noticed the update yet. Polling status.Replicas is not safe in the latter case.
852+
desiredGeneration := controller.Generation
853+
854+
return func() (bool, error) {
855+
ctrl, err := rcClient.ReplicationControllers(controller.Namespace).Get(controller.Name, metav1.GetOptions{})
856+
if err != nil {
857+
return false, err
858+
}
859+
// There's a chance a concurrent update modifies the Spec.Replicas causing this check to pass,
860+
// or, after this check has passed, a modification causes the rc manager to create more pods.
861+
// This will not be an issue once we've implemented graceful delete for rcs, but till then
862+
// concurrent stop operations on the same rc might have unintended side effects.
863+
return ctrl.Status.ObservedGeneration >= desiredGeneration && ctrl.Status.Replicas == valOrZero(ctrl.Spec.Replicas), nil
864+
}
865+
}

pkg/kubectl/rolling_updater_test.go renamed to pkg/kubectl/cmd/rollingupdate/rolling_updater_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package kubectl
17+
package rollingupdate
1818

1919
import (
2020
"bytes"
@@ -40,6 +40,7 @@ import (
4040
restclient "k8s.io/client-go/rest"
4141
manualfake "k8s.io/client-go/rest/fake"
4242
testcore "k8s.io/client-go/testing"
43+
"k8s.io/kubectl/pkg/scale"
4344
"k8s.io/kubectl/pkg/scheme"
4445
"k8s.io/kubectl/pkg/util"
4546
)
@@ -790,7 +791,7 @@ Scaling foo-v2 up to 2
790791
t.Logf("running test %d (%s) (up: %v, down: %v, oldReady: %v, newReady: %v)", i, tt.name, upTo, downTo, oldReady, newReady)
791792
updater := &RollingUpdater{
792793
ns: "default",
793-
scaleAndWait: func(rc *corev1.ReplicationController, retry *RetryParams, wait *RetryParams) (*corev1.ReplicationController, error) {
794+
scaleAndWait: func(rc *corev1.ReplicationController, retry *scale.RetryParams, wait *scale.RetryParams) (*corev1.ReplicationController, error) {
794795
// Return a scale up or scale down expectation depending on the rc,
795796
// and throw errors if there is no expectation expressed for this
796797
// call.
@@ -861,7 +862,7 @@ func TestUpdate_progressTimeout(t *testing.T) {
861862
newRc := newRc(0, 2)
862863
updater := &RollingUpdater{
863864
ns: "default",
864-
scaleAndWait: func(rc *corev1.ReplicationController, retry *RetryParams, wait *RetryParams) (*corev1.ReplicationController, error) {
865+
scaleAndWait: func(rc *corev1.ReplicationController, retry *scale.RetryParams, wait *scale.RetryParams) (*corev1.ReplicationController, error) {
865866
// Do nothing.
866867
return rc, nil
867868
},
@@ -906,7 +907,7 @@ func TestUpdate_assignOriginalAnnotation(t *testing.T) {
906907
rcClient: fake.CoreV1(),
907908
podClient: fake.CoreV1(),
908909
ns: "default",
909-
scaleAndWait: func(rc *corev1.ReplicationController, retry *RetryParams, wait *RetryParams) (*corev1.ReplicationController, error) {
910+
scaleAndWait: func(rc *corev1.ReplicationController, retry *scale.RetryParams, wait *scale.RetryParams) (*corev1.ReplicationController, error) {
910911
return rc, nil
911912
},
912913
getOrCreateTargetController: func(controller *corev1.ReplicationController, sourceID string) (*corev1.ReplicationController, bool, error) {

0 commit comments

Comments
 (0)