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

Bug 1781707: openshift-sdn, CNO: handle new kubeconfig path #420

Merged
merged 1 commit into from Dec 20, 2019
Merged

Bug 1781707: openshift-sdn, CNO: handle new kubeconfig path #420

merged 1 commit into from Dec 20, 2019

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Dec 10, 2019

We pull the apiserver url from the kubelet's kubeconfig. This has changed, and can be found in one of two places. Check them both.

Ultimately we need to generate this in a more sustainable way, but this is a good workaround (and easily backportable).

@openshift-ci-robot
Copy link
Contributor

@squeed: An error was encountered adding this pull request to the external tracker bugs for bug 1781707 on the Bugzilla server at https://bugzilla.redhat.com:

JSONRPC error 100500: Insecure dependency in parameter 3 of DBI::db=HASH(0x55b6fb1ab018)->do method call while running with -T switch at /loader/0x55b6f4cee870/Bugzilla/Extension/ExternalBugs/Bug.pm line 327.
at /loader/0x55b6f4cee870/Bugzilla/Extension/ExternalBugs/Bug.pm line 327.
Bugzilla::Extension::ExternalBugs::Bug::update_ext_info('Bugzilla::Extension::ExternalBugs::Bug=HASH(0x55b6fb421540)', 1) called at /loader/0x55b6f4cee870/Bugzilla/Extension/ExternalBugs/Bug.pm line 121
Bugzilla::Extension::ExternalBugs::Bug::create('Bugzilla::Extension::ExternalBugs::Bug', 'HASH(0x55b6fb3f3568)') called at /var/www/html/bugzilla/extensions/ExternalBugs/Extension.pm line 858
Bugzilla::Extension::ExternalBugs::bug_start_of_update('Bugzilla::Extension::ExternalBugs=HASH(0x55b6fb207600)', 'HASH(0x55b6fb3a1400)') called at /var/www/html/bugzilla/Bugzilla/Hook.pm line 21
Bugzilla::Hook::process('bug_start_of_update', 'HASH(0x55b6fb3a1400)') called at /var/www/html/bugzilla/Bugzilla/Bug.pm line 1170
Bugzilla::Bug::update('Bugzilla::Bug=HASH(0x55b6fb3a0058)') called at /loader/0x55b6f4cee870/Bugzilla/Extension/ExternalBugs/WebService.pm line 80
Bugzilla::Extension::ExternalBugs::WebService::add_external_bug('Bugzilla::WebService::Server::JSONRPC::Bugzilla::Extension::E...', 'HASH(0x55b6fb0ad910)') called at (eval 1985) line 1
eval ' $procedure->{code}->($self, @params)
;' called at /usr/share/perl5/vendor_perl/JSON/RPC/Legacy/Server.pm line 220
JSON::RPC::Legacy::Server::_handle('Bugzilla::WebService::Server::JSONRPC::Bugzilla::Extension::E...', 'HASH(0x55b6fb0c14f0)') called at /var/www/html/bugzilla/Bugzilla/WebService/Server/JSONRPC.pm line 295
Bugzilla::WebService::Server::JSONRPC::_handle('Bugzilla::WebService::Server::JSONRPC::Bugzilla::Extension::E...', 'HASH(0x55b6fb0c14f0)') called at /usr/share/perl5/vendor_perl/JSON/RPC/Legacy/Server.pm line 126
JSON::RPC::Legacy::Server::handle('Bugzilla::WebService::Server::JSONRPC::Bugzilla::Extension::E...') called at /var/www/html/bugzilla/Bugzilla/WebService/Server/JSONRPC.pm line 70
Bugzilla::WebService::Server::JSONRPC::handle('Bugzilla::WebService::Server::JSONRPC::Bugzilla::Extension::E...') called at /var/www/html/bugzilla/jsonrpc.cgi line 31
ModPerl::ROOT::Bugzilla::ModPerl::ResponseHandler::var_www_html_bugzilla_jsonrpc_2ecgi::handler('Apache2::RequestRec=SCALAR(0x55b6fb23c358)') called at /usr/lib64/perl5/vendor_perl/ModPerl/RegistryCooker.pm line 207
eval {...} called at /usr/lib64/perl5/vendor_perl/ModPerl/RegistryCooker.pm line 207
ModPerl::RegistryCooker::run('Bugzilla::ModPerl::ResponseHandler=HASH(0x55b6fb21d800)') called at /usr/lib64/perl5/vendor_perl/ModPerl/RegistryCooker.pm line 173
ModPerl::RegistryCooker::default_handler('Bugzilla::ModPerl::ResponseHandler=HASH(0x55b6fb21d800)') called at /usr/lib64/perl5/vendor_perl/ModPerl/Registry.pm line 32
ModPerl::Registry::handler('Bugzilla::ModPerl::ResponseHandler', 'Apache2::RequestRec=SCALAR(0x55b6fb23c358)') called at /var/www/html/bugzilla/mod_perl.pl line 139
Bugzilla::ModPerl::ResponseHandler::handler('Bugzilla::ModPerl::ResponseHandler', 'Apache2::RequestRec=SCALAR(0x55b6fb23c358)') called at (eval 1985) line 0
eval {...} called at (eval 1985) line 0

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

In response to this:

Bug 1781707: openshift-sdn, CNO: handle new kubeconfig path

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.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 10, 2019
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 10, 2019
@juanluisvaladas
Copy link
Contributor

/retest
/lgtm as a long as the location of the apiserver's kubeconfig doesn't change very often.
In the long run we probably want to have a standard for every component managed by the cluster operator.

@juanluisvaladas
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2019
Copy link
Contributor

@juanluisvaladas juanluisvaladas left a comment

Choose a reason for hiding this comment

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

I'm not very fond of having this defined in multiple places, but gets the job done.

@danwinship
Copy link
Contributor

Have you discussed this with whatever team moved the file so they know we're depending on it?

@danwinship
Copy link
Contributor

also maybe a good excuse to switch sdn to using KUBERNETES_SERVICE_HOST/KUBERNETES_SERVICE_PORT overrides? We ought to be able to just set those variables and then stop passing --url-only-kubeconfig, right?

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

15 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@squeed
Copy link
Contributor Author

squeed commented Dec 11, 2019

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 11, 2019
@squeed
Copy link
Contributor Author

squeed commented Dec 12, 2019

Bah:
open /var/lib/kubelet/kubeconfig: permission denied

@squeed
Copy link
Contributor Author

squeed commented Dec 13, 2019

weird, failed with no artifacts. either fluke or we really nuked the cluster.
/test e2e-gcp-upgrade

@squeed
Copy link
Contributor Author

squeed commented Dec 16, 2019

OK, now I'm really confused: the CNO is privileged and yet it's still getting permission denied.

@danwinship
Copy link
Contributor

selinux

@squeed
Copy link
Contributor Author

squeed commented Dec 16, 2019

selinux

Yup, but privileged: true should be sufficient. Bah.

We can't rely on the nodes having a kubeconfig where we used to expect
it. The masters still have it, though, so we can just get it in the CNO
and pass it to the SDN in a manifest.
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 20, 2019
@squeed
Copy link
Contributor Author

squeed commented Dec 20, 2019

Well, that's never going to pass. Looking at the manifests, the kubeconfig we expect should continue to exist on the masters. So, we can just get it from the CNO to fix the immediate in-the-wild bug.

So, undo the changes to the CNO, but still stop the SDN from reading kubeconfig.

@squeed
Copy link
Contributor Author

squeed commented Dec 20, 2019

gcp took a nap
/retest

@squeed
Copy link
Contributor Author

squeed commented Dec 20, 2019

filed SDN-740 to stop reading the kubeconfig.

@squeed
Copy link
Contributor Author

squeed commented Dec 20, 2019

still more gcp oddities
/retest

Copy link
Contributor

@pecameron pecameron left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juanluisvaladas, pecameron, squeed

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 0981500 into openshift:master Dec 20, 2019
@openshift-ci-robot
Copy link
Contributor

@squeed: All pull requests linked via external trackers have merged. Bugzilla bug 1781707 has been moved to the MODIFIED state.

In response to this:

Bug 1781707: openshift-sdn, CNO: handle new kubeconfig path

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.

@juanluisvaladas
Copy link
Contributor

/cherrypick release-4.3

@openshift-cherrypick-robot

@juanluisvaladas: new pull request created: #429

In response to this:

/cherrypick release-4.3

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.

@squeed
Copy link
Contributor Author

squeed commented Dec 20, 2019

Let's pick this to 4.2 too.

@juanluisvaladas
Copy link
Contributor

Before merging? OK
/cherrypick release-4.2

@openshift-cherrypick-robot

@juanluisvaladas: #420 failed to apply on top of branch "release-4.2":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	bindata/network/openshift-sdn/controller.yaml
M	bindata/network/openshift-sdn/sdn.yaml
Falling back to patching base and 3-way merge...
Auto-merging bindata/network/openshift-sdn/sdn.yaml
CONFLICT (content): Merge conflict in bindata/network/openshift-sdn/sdn.yaml
Auto-merging bindata/network/openshift-sdn/controller.yaml
Patch failed at 0001 sdn: Get apiserver URL from CNO, not from kubeconfig

In response to this:

Before merging? OK
/cherrypick release-4.2

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.

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. 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.

None yet

8 participants