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.

Sign up for GitHub

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

Jump to bottom

kubeadm: use 'gce' when dumping logs #11131

Merged
merged 1 commit into from
Feb 5, 2019
Merged

kubeadm: use 'gce' when dumping logs #11131

merged 1 commit into from
Feb 5, 2019

Conversation

neolit123
Copy link
Member

@neolit123 neolit123 commented Feb 5, 2019 •
edited
Loading

o_O

failures:
https://gubernator.k8s.io/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-gce-master/1798

I0204 22:19:38.408] E0204 22:19:38.408486    1865 test_context.go:430] Unknown provider "kubernetes-anywhere". The following providers are known: aws azure gce gke kubemark local skeleton

seems to me like the log dump is coupled with e2e and that's why it fails.
my last bet is to modify this line to be "gce" instead of "kubernetes-anywhere"
https://github.com/kubernetes/test-infra/blob/master/kubetest/anywhere.go#L280
as per:
https://github.com/kubernetes/kubernetes/blob/master/cluster/log-dump/log-dump.sh#L40-L42

caused by:
kubernetes/kubernetes#73402

/shrug
/assign @krzyzacy @BenTheElder

@k8s-ci-robot k8s-ci-robot added the ¯\_(ツ)_/¯ ¯\\\_(ツ)_/¯ label Feb 5, 2019
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. area/kubetest sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Feb 5, 2019
BenTheElder
BenTheElder reviewed Feb 5, 2019
View reviewed changes
kubetest/anywhere.go
@@ -277,7 +277,7 @@ func (k *kubernetesAnywhere) DumpClusterLogs(localPath, gcsPath string) error {
// not use log-dump.sh.
providerKey := "KUBERNETES_PROVIDER"
oldValue := os.Getenv(providerKey)
if err := os.Setenv(providerKey, "kubernetes-anywhere"); err != nil {
if err := os.Setenv(providerKey, "gce"); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

ahh this is fine

BenTheElder
BenTheElder approved these changes Feb 5, 2019
View reviewed changes
Copy link
Member

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
/pony fine

@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 42609142f61ffedc6ca4cadc8daebab201dc41bc

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 5, 2019
@k8s-ci-robot
Copy link
Contributor

@BenTheElder: pony image

In response to this:

/lgtm
/approve
/pony fine

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-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, neolit123

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 5, 2019
@k8s-ci-robot k8s-ci-robot merged commit c5afa43 into kubernetes:master Feb 5, 2019
@krzyzacy
Copy link
Member

krzyzacy commented Feb 5, 2019

/pony bespoke

@k8s-ci-robot
Copy link
Contributor

@krzyzacy: Couldn't find a pony matching that query.

In response to this:

/pony bespoke

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.

@krzyzacy
Copy link
Member

krzyzacy commented Feb 5, 2019

@neolit123 assume we need a global bump to fix CI as well?

@neolit123
Copy link
Member Author

@krzyzacy yep kubetest needs a bump.

@krzyzacy
Copy link
Member

krzyzacy commented Feb 5, 2019

on it

@krzyzacy krzyzacy mentioned this pull request Feb 5, 2019
Merged
@neolit123
Copy link
Member Author

neolit123 commented Feb 6, 2019 •
edited
Loading

didn't work:
https://gubernator.k8s.io/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-gce-master/1813

I0206 04:33:44.549] Conformance test: not doing test setup.
I0206 04:33:45.999] E0206 04:33:45.999258 1887 test_context.go:430] Unknown provider "kubernetes-anywhere". The following providers are known: aws azure gce gke kubemark local skeleton

i do not know where kubernetes-anywhere is coming from at this point.

also logdump started failing after this patch.
https://k8s-testgrid.appspot.com/sig-cluster-lifecycle-all#kubeadm-gce-master

so using gce in log-dump doesn't work even if it seems to me like kubernetes-anywhere is treated the same.
https://github.com/kubernetes/kubernetes/blob/master/cluster/log-dump/log-dump.sh

@krzyzacy
Copy link
Member

krzyzacy commented Feb 6, 2019

@neolit123 maybe instead of relying on k8s bash, we can try our homebrew e2e and dump packages? :-)

https://github.com/kubernetes/test-infra/tree/master/kubetest/e2e
and
https://github.com/kubernetes/test-infra/blob/master/kubetest/dump.go

@neolit123
Copy link
Member Author

ok, i'm going to try to use these:

W0206 14:32:06.535] JENKINS_GCE_SSH_PRIVATE_KEY_FILE=/workspace/.ssh/google_compute_engine
W0206 14:32:06.535] JENKINS_GCE_SSH_PUBLIC_KEY_FILE=/workspace/.ssh/google_compute_engine.pub

as private/pub key for fetching the logs.

and try the log dumping as implemented in kops.go:

test-infra/kubetest/kops.go

Line 452 in e0cf221

func (k kops) DumpClusterLogs(localPath, gcsPath string) error {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenTheElder BenTheElder BenTheElder approved these changes

@shyamjvs shyamjvs Awaiting requested review from shyamjvs

Assignees

@BenTheElder BenTheElder

@krzyzacy krzyzacy

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubetest cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. ¯\_(ツ)_/¯ ¯\\\_(ツ)_/¯
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@neolit123 @k8s-ci-robot @krzyzacy @BenTheElder