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

Change v1 API to default RC replicas to 1 #8552

Merged
merged 1 commit into from
May 22, 2015

Conversation

thockin
Copy link
Member

@thockin thockin commented May 20, 2015

Fixes #6700
Part of #7018

@thockin
Copy link
Member Author

thockin commented May 20, 2015

@bgrant0607 - chipping away at v1 nits

@bgrant0607
Copy link
Member

LGTM. Thanks much.
cc @rjnagal

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2015
@rjnagal
Copy link
Contributor

rjnagal commented May 20, 2015

Thanks @thockin. I was just trying to sent this out over bad conf wifi.

The test failure is real but looks unrelated:

@rjnagal
Copy link
Contributor

rjnagal commented May 20, 2015

ok github.com/GoogleCloudPlatform/kubernetes/pkg/securitycontext 0.045s coverage: 62.5% of statements
--- FAIL: TestNoManualChangesToGenerateConversions (0.54s)
conversion_generation_test.go:129: please update conversion functions; generated: v1.functions.txt; first diff:
expected: func convert_api_AWSElasticBlockStoreVolumeSource_To_v1_AWSElasticBlockStoreVolumeSource(in *api.AWSElasticBlockStoreVolumeSource, out *AWSElasticBlockStoreVolumeSource, s conversion.Scope) error {

         got: func convert_v1_AWSElasticBlockStoreVolumeSource_To_api_AWSElasticBlockStoreVolumeSource(in *AWSElasticBlockStoreVolumeSource, out *api.AWSElasticBlockStoreVolumeSource, s conversion.Scope) error {

FAIL

@thockin
Copy link
Member Author

thockin commented May 20, 2015

Sorry, didn't know you were on it.

'make test' passes here. weird.

@thockin
Copy link
Member Author

thockin commented May 20, 2015

rebased, let's see if that helps. It looks like it is complaining that the generated code is in a different order, but it passes on my desktop.

@wojtek-t is the conversion code maybe using a map somewhere?

@wojtek-t
Copy link
Member

this is strange - the code is supposed to sort the result:
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/runtime/conversion_generator.go#L243

@thockin
Copy link
Member Author

thockin commented May 20, 2015

Failed again:

--- FAIL: TestNoManualChangesToGenerateConversions (0.70s)
    conversion_generation_test.go:129: please update conversion functions; generated: v1.functions.txt; first diff: 
        expected: func convert_api_ObjectReference_To_v1_ObjectReference(in *api.ObjectReference, out *ObjectReference, s conversion.Scope) error {

             got: func convert_v1_ObjectReference_To_api_ObjectReference(in *ObjectReference, out *api.ObjectReference, s conversion.Scope) error {

I can not repro - it passes locally.

@bgrant0607 bgrant0607 removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2015
@wojtek-t
Copy link
Member

@thockin: I think the sorting function doesn't make sense (i.e. it's not deterministic). I will create a PR today to fix it).

@wojtek-t
Copy link
Member

@thockin #8577 that should fix your problems is out for review

@dchen1107
Copy link
Member

@thockin you have to rebase now. :-(

@thockin
Copy link
Member Author

thockin commented May 21, 2015

rebased

@dchen1107
Copy link
Member

@thockin you have to rebase again. Sorry for this.

@thockin
Copy link
Member Author

thockin commented May 21, 2015

rebased again

@dchen1107
Copy link
Member

We had e2e failures again after clean some backlogs for last two days. I am looking into those failures, and will merge this one first.

@thockin
Copy link
Member Author

thockin commented May 21, 2015

No worries.

On Thu, May 21, 2015 at 1:07 PM, Dawn Chen notifications@github.com wrote:

We had e2e failures again after clean some backlogs for last two days. I
am looking into those failures, and will merge this one first.


Reply to this email directly or view it on GitHub
#8552 (comment)
.

@thockin
Copy link
Member Author

thockin commented May 22, 2015

green and mergable :)

@rjnagal rjnagal added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2015
@rjnagal
Copy link
Contributor

rjnagal commented May 22, 2015

LGTM.

Oncall will merge when the door opens :)

@dchen1107
Copy link
Member

Yes, sorry for postponing on this. Merging...

dchen1107 added a commit that referenced this pull request May 22, 2015
Change v1 API to default RC replicas to 1
@dchen1107 dchen1107 merged commit 434a9ff into kubernetes:master May 22, 2015
@thockin thockin deleted the rc-default-1 branch June 25, 2015 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default ReplicationController replicas count to 1
6 participants