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

Support e2e testing via a bastion host for SSH commands #72286

Merged
merged 3 commits into from Jan 21, 2019

Conversation

@smarterclayton
Copy link
Contributor

smarterclayton commented Dec 21, 2018

Issue: e2e tests should succeed even if nodes are not publicly available from where the tests are run

Until we port the remaining e2e tests to use exec host pods instead of SSH, it should be possible to use bastion host when clusters don't expose public IPs for nodes. Testers can use the KUBE_SSH_BASTION environment variable set to a host:port to specify the host that the tests will use as a jump host.

If KUBE_SSH_BASTION is specified, the test will first connect to the bastion using SSH and the provider private key, then establish a tunnel from the bastion to the target node by its IP. If the node has an external IP, the external IP must be routable from the bastion.

Also support KUBE_SSH_KEY_PATH so there is a single cross-provider env var for setting the full path to the private key, which simplifies testing multiple clouds from the same infra.

/kind bug

e2e tests that require SSH may be used against clusters that have nodes without external IP addresses by setting the environment variable `KUBE_SSH_BASTION` to the `host:port` of a machine that is allowed to SSH to those nodes.  The same private key that the test would use is used for the bastion host.  The test connects to the bastion and then tunnels another SSH connection to the node.
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 21, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

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

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

smarterclayton commented Dec 21, 2018

@kubernetes/sig-testing-pr-reviews we have configurations that have private nodes and I would like to ensure that e2e tests running as prow jobs on a k8s cluster can reach into those other clusters to do the full e2e suite

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

smarterclayton commented Dec 21, 2018

/retest

1 similar comment
@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

smarterclayton commented Dec 22, 2018

/retest

smarterclayton added some commits Dec 21, 2018

Allow a single SSH key to be used in all providers
Support KUBE_SSH_KEY_PATH as a single environment variable that if
specified ignores the provider specific settinsg and loads the
SSH private key from the provided path. Makes it much easier to
specify a consistent signer across providers.
Support e2e testing via a bastion host for SSH commands
Until we port the remaining e2e tests to use exec host pods instead
of SSH, it should be possible to use bastion host when
clusters don't expose public IPs for nodes. Testers can use the
KUBE_SSH_BASTION environment variable set to a `host:port` to
specify the host that the tests will use as a jump host.

If KUBE_SSH_BASTION is specified, the test will first connect to
the bastion using SSH and the provider private key, then establish
a tunnel from the bastion to the target node by its IP. If the
node has an external IP, the external IP must be routable from the
bastion.

@smarterclayton smarterclayton force-pushed the smarterclayton:ssh branch from 8472568 to 88cac12 Jan 10, 2019

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

smarterclayton commented Jan 10, 2019

This is a more complete solution for #68792

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

smarterclayton commented Jan 11, 2019

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

smarterclayton commented Jan 11, 2019

/retest

@smarterclayton

This comment has been minimized.

Copy link
Contributor Author

smarterclayton commented Jan 16, 2019

@kubernetes/sig-node-pr-reviews need some eyes on this - nodes shouldn't be required to have external SSH access, and I think we're starting to get to the point where we shouldn't assume the e2e tests run from inside the security zone of a cluster. A bastion is reasonably simple with go-ssh (have been running this code in openshift for a while) and the code path is isolated so that if someone does hit bugs in this new path it doesn't impact others.

}
bastionClient, err := ssh.Dial("tcp", bastion, config)
if err != nil {
err = wait.Poll(5*time.Second, 20*time.Second, func() (bool, error) {

This comment has been minimized.

@rphillips

rphillips Jan 18, 2019

Member

would be nice to have the constants at the top of the file

This comment has been minimized.

@smarterclayton

smarterclayton Jan 21, 2019

Author Contributor

the code in this case is a copy of sshutil.RunSSHCommand and I'm trying to keep the structure of the method consistent (can't reuse the constant across two packages since we don't want to expose it)

err = fmt.Errorf("failed running `%s` on %s@%s: '%v'", cmd, user, host, err)
}
}
return bout.String(), berr.String(), code, err

This comment has been minimized.

@rphillips

rphillips Jan 18, 2019

Member

is it ok to buffer the entire stdout/stderr into memory? We may want to put the data into a limited buffer.

This comment has been minimized.

@smarterclayton

smarterclayton Jan 19, 2019

Author Contributor

This is no different than the other SSH stub, but generally yes, this is for test code only and the usage is constraint. We also dump the full output to the build logs, which is a disincentive to use this for a chatty channel.

So it's not great, but it's consistent with our existing flow, but in the future we should improve this structure (these are convenience commands that aren't really designed for anything other than quick and dirty testing).


func IssueSSHCommand(cmd, provider string, node *v1.Node) error {
_, err := IssueSSHCommandWithResult(cmd, provider, node)
if err != nil {

This comment has been minimized.

@rphillips

rphillips Jan 18, 2019

Member

just return err here

This comment has been minimized.

@smarterclayton

smarterclayton Jan 19, 2019

Author Contributor

this was a straight move, would prefer not to change the code.

@rphillips

This comment has been minimized.

Copy link
Member

rphillips commented Jan 21, 2019

/lgtm

@rphillips

This comment has been minimized.

Copy link
Member

rphillips commented Jan 21, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit c336229 into kubernetes:master Jan 21, 2019

18 checks passed

cla/linuxfoundation smarterclayton authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment