Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MinReadySeconds in deployment configs #9852

Merged
merged 3 commits into from
Jul 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/swagger-spec/oapi-v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -21548,6 +21548,11 @@
"$ref": "v1.DeploymentStrategy",
"description": "Strategy describes how a deployment is executed."
},
"minReadySeconds": {
"type": "integer",
"format": "int32",
"description": "MinReadySeconds is the minimum number of seconds for which a newly created pod should be ready without any of its container crashing, for it to be considered available. Defaults to 0 (pod will be considered available as soon as it is ready)"
},
"triggers": {
"type": "array",
"items": {
Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/cli/describe/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ func printDeploymentConfigSpec(kc kclient.Interface, dc deployapi.DeploymentConf
formatString(w, "Strategy", spec.Strategy.Type)
printStrategy(spec.Strategy, " ", w)

if dc.Spec.MinReadySeconds > 0 {
formatString(w, "MinReadySeconds", fmt.Sprintf("%d", spec.MinReadySeconds))
}

// Pod template
fmt.Fprintf(w, "Template:\n")
kctl.DescribePodTemplate(spec.Template, w)
Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/api/deep_copy_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ func DeepCopy_api_DeploymentConfigSpec(in DeploymentConfigSpec, out *DeploymentC
if err := DeepCopy_api_DeploymentStrategy(in.Strategy, &out.Strategy, c); err != nil {
return err
}
out.MinReadySeconds = in.MinReadySeconds
if in.Triggers != nil {
in, out := in.Triggers, &out.Triggers
*out = make([]DeploymentTriggerPolicy, len(in))
Expand Down
5 changes: 5 additions & 0 deletions pkg/deploy/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,11 @@ type DeploymentConfigSpec struct {
// Strategy describes how a deployment is executed.
Strategy DeploymentStrategy

// MinReadySeconds is the minimum number of seconds for which a newly created pod should
// be ready without any of its container crashing, for it to be considered available.
// Defaults to 0 (pod will be considered available as soon as it is ready)
MinReadySeconds int32

// Triggers determine how updates to a DeploymentConfig result in new deployments. If no triggers
// are defined, a new deployment can only occur as a result of an explicit client update to the
// DeploymentConfig with a new LatestVersion.
Expand Down
2 changes: 2 additions & 0 deletions pkg/deploy/api/v1/conversion_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ func autoConvert_v1_DeploymentConfigSpec_To_api_DeploymentConfigSpec(in *Deploym
if err := Convert_v1_DeploymentStrategy_To_api_DeploymentStrategy(&in.Strategy, &out.Strategy, s); err != nil {
return err
}
out.MinReadySeconds = in.MinReadySeconds
if in.Triggers != nil {
in, out := &in.Triggers, &out.Triggers
*out = make([]deploy_api.DeploymentTriggerPolicy, len(*in))
Expand Down Expand Up @@ -355,6 +356,7 @@ func autoConvert_api_DeploymentConfigSpec_To_v1_DeploymentConfigSpec(in *deploy_
if err := Convert_api_DeploymentStrategy_To_v1_DeploymentStrategy(&in.Strategy, &out.Strategy, s); err != nil {
return err
}
out.MinReadySeconds = in.MinReadySeconds
if in.Triggers != nil {
in, out := &in.Triggers, &out.Triggers
*out = make([]DeploymentTriggerPolicy, len(*in))
Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/api/v1/deep_copy_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ func DeepCopy_v1_DeploymentConfigSpec(in DeploymentConfigSpec, out *DeploymentCo
if err := DeepCopy_v1_DeploymentStrategy(in.Strategy, &out.Strategy, c); err != nil {
return err
}
out.MinReadySeconds = in.MinReadySeconds
if in.Triggers != nil {
in, out := in.Triggers, &out.Triggers
*out = make([]DeploymentTriggerPolicy, len(in))
Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/api/v1/swagger_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func (DeploymentConfigRollbackSpec) SwaggerDoc() map[string]string {
var map_DeploymentConfigSpec = map[string]string{
"": "DeploymentConfigSpec represents the desired state of the deployment.",
"strategy": "Strategy describes how a deployment is executed.",
"minReadySeconds": "MinReadySeconds is the minimum number of seconds for which a newly created pod should be ready without any of its container crashing, for it to be considered available. Defaults to 0 (pod will be considered available as soon as it is ready)",
"triggers": "Triggers determine how updates to a DeploymentConfig result in new deployments. If no triggers are defined, a new deployment can only occur as a result of an explicit client update to the DeploymentConfig with a new LatestVersion.",
"replicas": "Replicas is the number of desired replicas.",
"revisionHistoryLimit": "RevisionHistoryLimit is the number of old ReplicationControllers to retain to allow for rollbacks. This field is a pointer to allow for differentiation between an explicit zero and not specified.",
Expand Down
5 changes: 5 additions & 0 deletions pkg/deploy/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ type DeploymentConfigSpec struct {
// Strategy describes how a deployment is executed.
Strategy DeploymentStrategy `json:"strategy"`

// MinReadySeconds is the minimum number of seconds for which a newly created pod should
// be ready without any of its container crashing, for it to be considered available.
// Defaults to 0 (pod will be considered available as soon as it is ready)
MinReadySeconds int32 `json:"minReadySeconds,omitempty"`

// Triggers determine how updates to a DeploymentConfig result in new deployments. If no triggers
// are defined, a new deployment can only occur as a result of an explicit client update to the
// DeploymentConfig with a new LatestVersion.
Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func ValidateDeploymentConfigSpec(spec deployapi.DeploymentConfigSpec) field.Err
if spec.RevisionHistoryLimit != nil {
allErrs = append(allErrs, kapivalidation.ValidateNonnegativeField(int64(*spec.RevisionHistoryLimit), specPath.Child("revisionHistoryLimit"))...)
}
allErrs = append(allErrs, kapivalidation.ValidateNonnegativeField(int64(spec.MinReadySeconds), specPath.Child("minReadySeconds"))...)
if spec.Template == nil {
allErrs = append(allErrs, field.Required(specPath.Child("template"), ""))
} else {
Expand Down
4 changes: 1 addition & 3 deletions pkg/deploy/controller/deploymentconfig/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,12 @@ func (c *DeploymentConfigController) updateStatus(config *deployapi.DeploymentCo
}

func (c *DeploymentConfigController) calculateStatus(config deployapi.DeploymentConfig, deployments []kapi.ReplicationController) (deployapi.DeploymentConfigStatus, error) {
// TODO: Implement MinReadySeconds for deploymentconfigs: https://github.com/openshift/origin/issues/7114
minReadSeconds := int32(0)
selector := labels.Set(config.Spec.Selector).AsSelector()
pods, err := c.podStore.Pods(config.Namespace).List(selector)
if err != nil {
return config.Status, err
}
available := deployutil.GetAvailablePods(pods.Items, minReadSeconds)
available := deployutil.GetAvailablePods(pods.Items, config.Spec.MinReadySeconds)

// UpdatedReplicas represents the replicas that use the deployment config template which means
// we should inform about the replicas of the latest deployment and not the active.
Expand Down
8 changes: 4 additions & 4 deletions pkg/deploy/strategy/recreate/recreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type RecreateDeploymentStrategy struct {
getReplicationController func(namespace, name string) (*kapi.ReplicationController, error)
// getUpdateAcceptor returns an UpdateAcceptor to verify the first replica
// of the deployment.
getUpdateAcceptor func(timeout time.Duration) strat.UpdateAcceptor
getUpdateAcceptor func(time.Duration, int32) strat.UpdateAcceptor
// scaler is used to scale replication controllers.
scaler kubectl.Scaler
// tagClient is used to tag images
Expand Down Expand Up @@ -72,8 +72,8 @@ func NewRecreateDeploymentStrategy(client kclient.Interface, tagClient client.Im
getReplicationController: func(namespace, name string) (*kapi.ReplicationController, error) {
return client.ReplicationControllers(namespace).Get(name)
},
getUpdateAcceptor: func(timeout time.Duration) strat.UpdateAcceptor {
return stratsupport.NewAcceptNewlyObservedReadyPods(out, client, timeout, AcceptorInterval)
getUpdateAcceptor: func(timeout time.Duration, minReadySeconds int32) strat.UpdateAcceptor {
return stratsupport.NewAcceptNewlyObservedReadyPods(out, client, timeout, AcceptorInterval, minReadySeconds)
},
scaler: scaler,
decoder: decoder,
Expand Down Expand Up @@ -106,7 +106,7 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo
waitParams := kubectl.NewRetryParams(s.retryPeriod, s.retryTimeout)

if updateAcceptor == nil {
updateAcceptor = s.getUpdateAcceptor(time.Duration(*params.TimeoutSeconds) * time.Second)
updateAcceptor = s.getUpdateAcceptor(time.Duration(*params.TimeoutSeconds)*time.Second, config.Spec.MinReadySeconds)
}

// Execute any pre-hook.
Expand Down
2 changes: 1 addition & 1 deletion pkg/deploy/strategy/recreate/recreate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ func (t *testControllerClient) updateReplicationController(namespace string, ctr
return t.updateReplicationControllerFunc(namespace, ctrl)
}

func getUpdateAcceptor(timeout time.Duration) strategy.UpdateAcceptor {
func getUpdateAcceptor(timeout time.Duration, minReadySeconds int32) strategy.UpdateAcceptor {
return &testAcceptor{
acceptFn: func(deployment *kapi.ReplicationController) error {
return nil
Expand Down
27 changes: 14 additions & 13 deletions pkg/deploy/strategy/rolling/rolling.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ type RollingDeploymentStrategy struct {
hookExecutor hookExecutor
// getUpdateAcceptor returns an UpdateAcceptor to verify the first replica
// of the deployment.
getUpdateAcceptor func(timeout time.Duration) strat.UpdateAcceptor
getUpdateAcceptor func(time.Duration, int32) strat.UpdateAcceptor
// apiRetryPeriod is how long to wait before retrying a failed API call.
apiRetryPeriod time.Duration
// apiRetryTimeout is how long to retry API calls before giving up.
Expand Down Expand Up @@ -106,8 +106,8 @@ func NewRollingDeploymentStrategy(namespace string, client kclient.Interface, ta
return updater.Update(config)
},
hookExecutor: stratsupport.NewHookExecutor(client, tags, events, os.Stdout, decoder),
getUpdateAcceptor: func(timeout time.Duration) strat.UpdateAcceptor {
return stratsupport.NewAcceptNewlyObservedReadyPods(out, client, timeout, AcceptorInterval)
getUpdateAcceptor: func(timeout time.Duration, minReadySeconds int32) strat.UpdateAcceptor {
return stratsupport.NewAcceptNewlyObservedReadyPods(out, client, timeout, AcceptorInterval, minReadySeconds)
},
}
}
Expand All @@ -119,7 +119,7 @@ func (s *RollingDeploymentStrategy) Deploy(from *kapi.ReplicationController, to
}

params := config.Spec.Strategy.RollingParams
updateAcceptor := s.getUpdateAcceptor(time.Duration(*params.TimeoutSeconds) * time.Second)
updateAcceptor := s.getUpdateAcceptor(time.Duration(*params.TimeoutSeconds)*time.Second, config.Spec.MinReadySeconds)

// If there's no prior deployment, delegate to another strategy since the
// rolling updater only supports transitioning between two deployments.
Expand Down Expand Up @@ -216,15 +216,16 @@ func (s *RollingDeploymentStrategy) Deploy(from *kapi.ReplicationController, to

// Perform a rolling update.
rollingConfig := &kubectl.RollingUpdaterConfig{
Out: &rollingUpdaterWriter{w: s.out},
OldRc: from,
NewRc: to,
UpdatePeriod: time.Duration(*params.UpdatePeriodSeconds) * time.Second,
Interval: time.Duration(*params.IntervalSeconds) * time.Second,
Timeout: time.Duration(*params.TimeoutSeconds) * time.Second,
CleanupPolicy: kubectl.PreserveRollingUpdateCleanupPolicy,
MaxSurge: params.MaxSurge,
MaxUnavailable: params.MaxUnavailable,
Out: &rollingUpdaterWriter{w: s.out},
OldRc: from,
NewRc: to,
UpdatePeriod: time.Duration(*params.UpdatePeriodSeconds) * time.Second,
Interval: time.Duration(*params.IntervalSeconds) * time.Second,
Timeout: time.Duration(*params.TimeoutSeconds) * time.Second,
MinReadySeconds: config.Spec.MinReadySeconds,
CleanupPolicy: kubectl.PreserveRollingUpdateCleanupPolicy,
MaxSurge: params.MaxSurge,
MaxUnavailable: params.MaxUnavailable,
OnProgress: func(oldRc, newRc *kapi.ReplicationController, percentage int) error {
if expect, ok := strat.Percentage(s.until); ok && percentage >= expect {
return strat.NewConditionReachedErr(fmt.Sprintf("Reached %s (currently %d%%)", s.until, percentage))
Expand Down
2 changes: 1 addition & 1 deletion pkg/deploy/strategy/rolling/rolling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func rollingParams(preFailurePolicy, postFailurePolicy deployapi.LifecycleHookFa
}
}

func getUpdateAcceptor(timeout time.Duration) strat.UpdateAcceptor {
func getUpdateAcceptor(timeout time.Duration, minReadySeconds int32) strat.UpdateAcceptor {
return &testAcceptor{
acceptFn: func(deployment *kapi.ReplicationController) error {
return nil
Expand Down
25 changes: 19 additions & 6 deletions pkg/deploy/strategy/support/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"k8s.io/kubernetes/pkg/fields"
"k8s.io/kubernetes/pkg/labels"
"k8s.io/kubernetes/pkg/runtime"
kdeployutil "k8s.io/kubernetes/pkg/util/deployment"
utilerrors "k8s.io/kubernetes/pkg/util/errors"
"k8s.io/kubernetes/pkg/util/sets"
"k8s.io/kubernetes/pkg/util/wait"
Expand Down Expand Up @@ -486,12 +487,20 @@ func NewPodWatch(client kclient.PodsNamespacer, namespace, name, resourceVersion

// NewAcceptNewlyObservedReadyPods makes a new AcceptNewlyObservedReadyPods
// from a real client.
func NewAcceptNewlyObservedReadyPods(out io.Writer, kclient kclient.PodsNamespacer, timeout time.Duration, interval time.Duration) *AcceptNewlyObservedReadyPods {
func NewAcceptNewlyObservedReadyPods(
out io.Writer,
kclient kclient.PodsNamespacer,
timeout time.Duration,
interval time.Duration,
minReadySeconds int32,
) *AcceptNewlyObservedReadyPods {

return &AcceptNewlyObservedReadyPods{
out: out,
timeout: timeout,
interval: interval,
acceptedPods: sets.NewString(),
out: out,
timeout: timeout,
interval: interval,
minReadySeconds: minReadySeconds,
acceptedPods: sets.NewString(),
getDeploymentPodStore: func(deployment *kapi.ReplicationController) (cache.Store, chan struct{}) {
selector := labels.Set(deployment.Spec.Selector).AsSelector()
store := cache.NewStore(cache.MetaNamespaceKeyFunc)
Expand Down Expand Up @@ -536,6 +545,10 @@ type AcceptNewlyObservedReadyPods struct {
timeout time.Duration
// interval is how often to check for pod readiness
interval time.Duration
// minReadySeconds is the minimum number of seconds for which a newly created
// pod should be ready without any of its container crashing, for it to be
// considered available.
minReadySeconds int32
// acceptedPods keeps track of pods which have been previously accepted for
// a deployment.
acceptedPods sets.String
Expand Down Expand Up @@ -563,7 +576,7 @@ func (c *AcceptNewlyObservedReadyPods) Accept(deployment *kapi.ReplicationContro
if c.acceptedPods.Has(pod.Name) {
continue
}
if kapi.IsPodReady(pod) {
if kdeployutil.IsPodAvailable(pod, c.minReadySeconds, time.Now()) {
// If the pod is ready, track it as accepted.
c.acceptedPods.Insert(pod.Name)
} else {
Expand Down
3 changes: 2 additions & 1 deletion pkg/deploy/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"sort"
"strconv"
"strings"
"time"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/labels"
Expand Down Expand Up @@ -272,7 +273,7 @@ func GetAvailablePods(pods []api.Pod, minReadySeconds int32) int32 {
available := int32(0)
for i := range pods {
pod := pods[i]
if kdeplutil.IsPodAvailable(&pod, minReadySeconds) {
if kdeplutil.IsPodAvailable(&pod, minReadySeconds, time.Now()) {
available++
}
}
Expand Down
49 changes: 49 additions & 0 deletions test/extended/deployments/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
o "github.com/onsi/gomega"

kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/labels"
"k8s.io/kubernetes/pkg/util/wait"
e2e "k8s.io/kubernetes/test/e2e/framework"

Expand All @@ -31,6 +32,7 @@ var _ = g.Describe("deploymentconfigs", func() {
failedHookFixture = exutil.FixturePath("testdata", "failing-pre-hook.yaml")
brokenDeploymentFixture = exutil.FixturePath("testdata", "test-deployment-broken.yaml")
historyLimitedDeploymentFixture = exutil.FixturePath("testdata", "deployment-history-limit.yaml")
minReadySecondsFixture = exutil.FixturePath("testdata", "deployment-min-ready-seconds.yaml")
)

g.Describe("when run iteratively", func() {
Expand Down Expand Up @@ -316,6 +318,7 @@ var _ = g.Describe("deploymentconfigs", func() {
one := int64(1)
config.Spec.Template.Spec.TerminationGracePeriodSeconds = &one
_, err = oc.REST().DeploymentConfigs(oc.Namespace()).Update(config)
// TODO: Retry on update conflicts
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(waitForLatestCondition(oc, name, deploymentRunTimeout, deploymentReachedCompletion)).NotTo(o.HaveOccurred())

Expand Down Expand Up @@ -551,4 +554,50 @@ var _ = g.Describe("deploymentconfigs", func() {
}
})
})

g.Describe("with minimum ready seconds set", func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how fast this test is? should this be a "fast" test or marked as "slow" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As fast as the pods transition to Ready - so it should be fast enough

g.It("should include various info in status [Conformance]", func() {
_, name, err := createFixture(oc, minReadySecondsFixture)
o.Expect(err).NotTo(o.HaveOccurred())

g.By("verifying the deployment is marked running")
o.Expect(waitForLatestCondition(oc, name, deploymentRunTimeout, deploymentRunning)).NotTo(o.HaveOccurred())

g.By("verifying that all pods are ready")
config, err := oc.REST().DeploymentConfigs(oc.Namespace()).Get(name)
o.Expect(err).NotTo(o.HaveOccurred())

selector := labels.Set(config.Spec.Selector).AsSelector()
opts := kapi.ListOptions{LabelSelector: selector}
ready := 0
if err := wait.PollImmediate(500*time.Millisecond, 1*time.Minute, func() (bool, error) {
pods, err := oc.KubeREST().Pods(oc.Namespace()).List(opts)
if err != nil {
return false, nil
}

ready = 0
for i := range pods.Items {
pod := pods.Items[i]
if kapi.IsPodReady(&pod) {
ready++
}
}

return len(pods.Items) == ready, nil
}); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this if and use o.Expect(err).NotTo(o.HaveOccurred())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err is a WaitTimeoutError ("timedout waiting for the condition") so in case it is non-nil I need to override with a more meaningful error.

o.Expect(fmt.Errorf("deployment config %q never became ready (ready: %d, desired: %d)",
config.Name, ready, config.Spec.Replicas)).NotTo(o.HaveOccurred())
}

g.By("verifying that the deployment is still running")
latestName := deployutil.DeploymentNameForConfigVersion(name, config.Status.LatestVersion)
latest, err := oc.KubeREST().ReplicationControllers(oc.Namespace()).Get(latestName)
o.Expect(err).NotTo(o.HaveOccurred())
if deployutil.IsTerminatedDeployment(latest) {
o.Expect(fmt.Errorf("expected deployment %q not to have terminated", latest.Name)).NotTo(o.HaveOccurred())
}
o.Expect(waitForLatestCondition(oc, name, deploymentRunTimeout, deploymentRunning)).NotTo(o.HaveOccurred())
})
})
})
22 changes: 22 additions & 0 deletions test/extended/testdata/deployment-min-ready-seconds.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
apiVersion: v1
kind: DeploymentConfig
metadata:
name: minreadytest
spec:
replicas: 2
minReadySeconds: 1000
selector:
name: minreadytest
template:
metadata:
labels:
name: minreadytest
spec:
terminationGracePeriodSeconds: 0
containers:
- image: "docker.io/centos:centos7"
imagePullPolicy: IfNotPresent
name: myapp
command:
- /bin/sleep
- "10000"
Loading