Skip to content

Comments

Don't allow scaling of in progress k8s deployments#733

Merged
openshift-bot merged 2 commits intoopenshift:masterfrom
spadgett:disable-scaling-rolling-deployments
Oct 26, 2016
Merged

Don't allow scaling of in progress k8s deployments#733
openshift-bot merged 2 commits intoopenshift:masterfrom
spadgett:disable-scaling-rolling-deployments

Conversation

@spadgett
Copy link
Member

// deployment selector with active replicas.
var numActive = 0;
_.each(replicaSets.by('metadata.name'), function(replicaSet) {
if (!deploymentSelector.matches(replicaSet) || !replicaSet.status.replicas) {
Copy link
Member

Choose a reason for hiding this comment

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

is matches the right check here? shouldn't it be comparing the labelSelector of the deployment to the labelSelector of the replicaSet? matches is going to checks the labels on the replicaSet, covers would check the labelSelector

@spadgett spadgett force-pushed the disable-scaling-rolling-deployments branch from 4a72678 to 2c9fa8c Compare October 26, 2016 14:15
@spadgett
Copy link
Member Author

@jwforres thanks, updated

@jwforres
Copy link
Member

@spadgett were you using matches anywhere else in the deployment/replicaset rollup that we should have been using covers instead?

@jwforres
Copy link
Member

actually i see you updated another one in your latest commit, thanks [merge]

@openshift-bot
Copy link

Evaluated for origin web console merge up to 2c9fa8c

@openshift-bot
Copy link

openshift-bot commented Oct 26, 2016

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/577/) (Base Commit: 9ba5318)

@openshift-bot openshift-bot merged commit 98cd377 into openshift:master Oct 26, 2016
@spadgett spadgett deleted the disable-scaling-rolling-deployments branch October 26, 2016 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants