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

UPSTREAM: 16668: Fix hpa escalation #5579

Merged
merged 1 commit into from
Nov 4, 2015

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Nov 2, 2015

pick for kubernetes/kubernetes#16661

@liggitt @smarterclayton we need this (or something like it) to make the HPA controller safe in 3.1
@DirectXMan12 @ncdc This is needed in or before #5310

@smarterclayton
Copy link
Contributor

[test]

@smarterclayton smarterclayton added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 2, 2015
@smarterclayton
Copy link
Contributor

I would recommend just carrying the changes that ignore the namespace on the field, rather than changing the kube object. If someone specifies the namespace, we merely ignore it. That reduces the risk of an upstream change.

@smarterclayton
Copy link
Contributor

You have permission to carry the change that ignores Namespace without argument without upstream.

@deads2k
Copy link
Contributor Author

deads2k commented Nov 2, 2015

I would recommend just carrying the changes that ignore the namespace on the field, rather than changing the kube object. If someone specifies the namespace, we merely ignore it. That reduces the risk of an upstream change.

I can do that, but the risk during rebase is high because the old field will still be referenceable.

@deads2k
Copy link
Contributor Author

deads2k commented Nov 2, 2015

setup problem re[test]

@liggitt
Copy link
Contributor

liggitt commented Nov 2, 2015

I don't want to leave a dangerous latent field in play for another controller to start using

@liggitt
Copy link
Contributor

liggitt commented Nov 2, 2015

upstream has LGTM

@liggitt
Copy link
Contributor

liggitt commented Nov 2, 2015

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6593/

!!! Error in hack/../test/end-to-end/core.sh:156
    '[ "$(oc rsh ${BUILD_ID}-build 2>&1 | grep 'forbidden')" ]' exited with status 1
Call stack:
    1: hack/../test/end-to-end/core.sh:156 main(...)

[test]

@liggitt
Copy link
Contributor

liggitt commented Nov 3, 2015

[test]

@liggitt
Copy link
Contributor

liggitt commented Nov 3, 2015

#4351

error: deploymentConfig "test-deployment-config" cannot be updated: the object has been modified; please apply your changes to the latest version and try again
!!! Error in test/cmd/volumes.sh:17
    '[ "$(oc volume dc/test-deployment-config --add --name=vol2 --type=emptydir -m /opt)" ]' exited with status 1

[test]

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2015
@deads2k
Copy link
Contributor Author

deads2k commented Nov 3, 2015

[merge]

@deads2k deads2k removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2015
@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to db848a0

@smarterclayton
Copy link
Contributor

[test]

1 similar comment
@smarterclayton
Copy link
Contributor

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to db848a0

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6834/) (Image: devenv-rhel7_2638)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6834/)

openshift-bot pushed a commit that referenced this pull request Nov 4, 2015
@openshift-bot openshift-bot merged commit ee93b8d into openshift:master Nov 4, 2015
@pweil- pweil- mentioned this pull request Nov 4, 2015
@deads2k deads2k deleted the fix-hpa-attack branch November 5, 2015 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. priority/P0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants