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

Automated cherry pick of #49495 upstream release 1.7 #49767

Conversation

wojtek-t
Copy link
Member

Cherry pick of #49495 on release-1.7.

#49495: make it possible to allow discovery errors for controllers

@wojtek-t wojtek-t added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 28, 2017
@wojtek-t wojtek-t added this to the v1.7 milestone Jul 28, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 28, 2017
@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 28, 2017
@wojtek-t
Copy link
Member Author

/test pull-kubernetes-unit

1 similar comment
@wojtek-t
Copy link
Member Author

/test pull-kubernetes-unit

@wojtek-t
Copy link
Member Author

@deads2k - it seems those failures are not flakes. Can you please take a look?

@deads2k
Copy link
Contributor

deads2k commented Jul 28, 2017

@deads2k - it seems those failures are not flakes. Can you please take a look?

Looks like an incomplete federation stub of some kind. Maybe they can just do whatever they change in 1.8.

@wojtek-t
Copy link
Member Author

@deads2k - this is not failing on 1.7 branch and on other cherrypicks. This is related to this particular PR.

@wojtek-t
Copy link
Member Author

/retest

3 similar comments
@wojtek-t
Copy link
Member Author

wojtek-t commented Aug 1, 2017

/retest

@wojtek-t
Copy link
Member Author

wojtek-t commented Aug 7, 2017

/retest

@wojtek-t
Copy link
Member Author

/retest

@foxish
Copy link
Contributor

foxish commented Aug 15, 2017

/test pull-kubernetes-unit

@foxish
Copy link
Contributor

foxish commented Aug 15, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2017
@foxish
Copy link
Contributor

foxish commented Aug 15, 2017

@kubernetes/sig-federation-bugs @nikhiljindal PTAL.
This fix is important for 1.7, and appears blocked by the failing unit test.

@k8s-ci-robot k8s-ci-robot added sig/federation kind/bug Categorizes issue or PR as related to a bug. labels Aug 15, 2017
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

1 similar comment
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

@wojtek-t
Copy link
Member Author

@foxish - I'm pretty convinced that failing unit tests are related to this cl - I have seen this failure on any other PR to 1.7 branch and on this one I've seen them 5+ times

@foxish
Copy link
Contributor

foxish commented Aug 16, 2017

Sorry, I see that now. I was thrown off by @deads2k's comment. Investigating further.

@foxish foxish removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 16, 2017
@foxish
Copy link
Contributor

foxish commented Aug 16, 2017

Looks like these tests were deleted entirely in master in #47890.

@irfanurrehman
Copy link
Contributor

@kubernetes/sig-federation-bugs @nikhiljindal PTAL.
This fix is important for 1.7, and appears blocked by the failing unit test.

Spent some time on this, and raised #50771 to still let those federated namespace unit tests run ok with the changes in this cherry-pick PR.

Looks like these tests were deleted entirely in master in #47890.

@foxish you are right, we have done away with these tests completely and no longer exist on master (thanks to @maruns efforts). In the tests which still exist on 1.7, the tests actually do not at all use the discovery functions, but there is no faked method which can bypass the checks introduced in this PR during initialising the controller. This cherry-pick should work if its put on top of the PR that I submitted.
cc @nikhiljindal .

@wojtek-t
Copy link
Member Author

/retest

@wojtek-t
Copy link
Member Author

/lgtm

@k8s-ci-robot
Copy link
Contributor

@wojtek-t: you cannot LGTM your own PR.

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: foxish, wojtek-t

Associated issue: 49495

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@wojtek-t wojtek-t added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 17, 2017
@wojtek-t
Copy link
Member Author

/retest

1 similar comment
@wojtek-t
Copy link
Member Author

/retest

@wojtek-t wojtek-t added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Aug 17, 2017
@wojtek-t
Copy link
Member Author

I'm not convinced that those flakes aren't real issues - those are to frequent comparing to other PRs.
Also it's pretty late and we really want to cut the release today.
So I'm declining to take this PR into 1.7.4 - we should try to let this in for 1.7.5 (we can consider another patch release next week if this is so important).

@foxish
Copy link
Contributor

foxish commented Aug 17, 2017

/test pull-kubernetes-e2e-gce-etcd3

They don't look related to the change though.

@foxish
Copy link
Contributor

foxish commented Aug 17, 2017

Different tests failing each time, which make me suspect it is indeed a flake. Retesting.
/test pull-kubernetes-e2e-gce-etcd3

@foxish foxish removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Aug 17, 2017
@foxish
Copy link
Contributor

foxish commented Aug 17, 2017

@wojtek-t, removed do-not-merge. 1.7.4 is cut already as I understand. Targeting this for 1.7.5.

@foxish
Copy link
Contributor

foxish commented Aug 17, 2017

/test pull-kubernetes-e2e-gce-etcd3

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

2 similar comments
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 423592b into kubernetes:release-1.7 Aug 18, 2017
@wojtek-t wojtek-t deleted the automated-cherry-pick-of-#49495-upstream-release-1.7 branch February 1, 2018 13:39
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. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants