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

Revert back to the "normal" apiserver authentication #15739

Merged
merged 1 commit into from
Jan 11, 2018

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Aug 10, 2017

@openshift-merge-robot openshift-merge-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. retest-not-required-docs-only approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 10, 2017
@deads2k deads2k force-pushed the tsb-15-revert-authn branch from 61af87a to 950b29c Compare August 11, 2017 11:44
@openshift-merge-robot openshift-merge-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. retest-not-required-docs-only labels Aug 11, 2017
@openshift-merge-robot openshift-merge-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 30, 2017
@openshift-merge-robot openshift-merge-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 28, 2017
@bparees
Copy link
Contributor

bparees commented Oct 4, 2017

@deads2k the PR this was blocked on has been closed, what's the latest status?

@deads2k deads2k force-pushed the tsb-15-revert-authn branch from 950b29c to 7b09621 Compare October 4, 2017 17:52
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2017
@deads2k
Copy link
Contributor Author

deads2k commented Oct 4, 2017

@bparees rebased

@bparees
Copy link
Contributor

bparees commented Oct 4, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, deads2k

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

@bparees bparees added kind/bug Categorizes issue or PR as related to a bug. and removed do-not-merge labels Oct 4, 2017
@deads2k
Copy link
Contributor Author

deads2k commented Oct 4, 2017

/retest

1 similar comment
@deads2k
Copy link
Contributor Author

deads2k commented Oct 4, 2017

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Oct 4, 2017

@bparees @jim-minter can you guys put the test back to the way I initially built it, where the daemonset and service were located in the namespace that the ansible installer was going to lay them into? Alternatively, you can try to wire your roles up properly.

@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from openshift-bot Oct 12, 2017
@openshift openshift deleted a comment from 0xmichalis Oct 12, 2017
@bparees
Copy link
Contributor

bparees commented Oct 23, 2017

@deads2k openshift/openshift-ansible#5056 has closed, is this still blocked on anything?

@deads2k
Copy link
Contributor Author

deads2k commented Oct 23, 2017

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Oct 23, 2017

/test all

@bparees
Copy link
Contributor

bparees commented Nov 9, 2017

@deads2k i'm still not clear on if anything is holding this up and/or if there are still test changes that need(or should) to go with this

@deads2k
Copy link
Contributor Author

deads2k commented Nov 9, 2017

@deads2k i'm still not clear on if anything is holding this up and/or if there are still test changes that need(or should) to go with this

Nothing needed for 3.7.

@bparees
Copy link
Contributor

bparees commented Nov 9, 2017

talked to @deads2k and this is failing for reasons that are unclear w/o being able to get TSB logs.

we're hoping that once @jim-minter's work to use the cluster-wide TSB installed by ansible, in extended tests, it will be easier to get the right logs to see why this is failing.

(@deads2k contends that the TSB is crashing, and i'm not sure it'll be any easier to get the logs if that's true, but i guess we'll cross that bridge when we get to it).

in the meantime:
/retest

@bparees
Copy link
Contributor

bparees commented Nov 9, 2017

also to follow up on the request to revert the TSB tests: At some point @deads2k wrote the TSB tests to share an TSB installed at the cluster level (which would align w/ how things will work once @jim-minter's effort to get us using the ansible installed TSB lands). At some future point, the tests were reworked to use a TSB instance per test. I do not recall the change or the reasoning for it, though I do recall a number of flakes in the tests and some iteration to resolve them.

In any case, once @jim-minter's change lands to be able to use the ansible installed TSB, we will be changing the tests back to not using their own TSB, so @deads2k will get his wish.

@deads2k also expressed concern that when we go to a single TSB, our tests may start conflicting. I think we're safe from that since our tests do not manipulate the catalog, and everything they do do is done in a test-scoped namespace (provision/deprovision).

But if we ever did introduce tests that need to register new templates(or worse, de-register some), or change which namespaces the TSB monitors for templates to offer in the catalog, we'll need to keep that concern in mind.

@bparees
Copy link
Contributor

bparees commented Nov 28, 2017

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Jan 11, 2018

/test all

@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Jan 11, 2018
@deads2k
Copy link
Contributor Author

deads2k commented Jan 11, 2018

bump bot

@openshift-merge-robot
Copy link
Contributor

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

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 15739, 18060).

@openshift-merge-robot openshift-merge-robot merged commit 8043bc0 into openshift:master Jan 11, 2018
@openshift-ci-robot
Copy link

@deads2k: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/origin/verify 7b09621 link /test origin-verify
ci/openshift-jenkins/origin/unit 7b09621 link /test origin-ut
ci/openshift-jenkins/extended_conformance_install_update 7b09621 link /test extended_conformance_install_update
ci/openshift-jenkins/extended_conformance_gce 7b09621 link /test extended_conformance_gce
ci/openshift-jenkins/extended_conformance_install 7b09621 link /test extended_conformance_install

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@deads2k deads2k deleted the tsb-15-revert-authn branch January 24, 2018 14:35
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. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants