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

Libssh in provision #1261

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

aerosouund
Copy link
Member

What this PR does / why we need it:

Removes occurrences of ssh.sh from the provision command in the gocli, this should have the benefit of ending the timeout during shutdown flake since ssh connection is made directly to the VM without an intermediary

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Release note:

Use libssh in provision cmd

@kubevirt-bot kubevirt-bot requested a review from ormergi August 30, 2024 10:19
@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Aug 30, 2024
@kubevirt-bot kubevirt-bot requested a review from qinqon August 30, 2024 10:19
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign enp0s3 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@aerosouund
Copy link
Member Author

cc: @dhiller
This introduces the fix i mentioned in the last meeting

@aerosouund
Copy link
Member Author

/test all

@@ -38,7 +39,7 @@ func NewProvisionCommand() *cobra.Command {
provision.Flags().StringP("memory", "m", "3096M", "amount of ram per node")
provision.Flags().UintP("cpu", "c", 2, "number of cpu cores per node")
provision.Flags().String("qemu-args", "", "additional qemu args to pass through to the nodes")
provision.Flags().Bool("random-ports", false, "expose all ports on random localhost ports")
provision.Flags().Bool("random-ports", true, "expose all ports on random localhost ports")
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why we have this flag, @brianmcarey ?

@@ -241,7 +257,7 @@ func provisionCluster(cmd *cobra.Command, args []string) (retErr error) {

envVars := fmt.Sprintf("version=%s slim=%t", version, slim)
if strings.Contains(phases, "linux") {
err = performPhase(cli, nodeContainer(prefix, nodeName), "/scripts/provision.sh", envVars)
err := sshClient.Command("sudo " + envVars + " /bin/bash /scripts/provision.sh")
Copy link
Member

Choose a reason for hiding this comment

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

Are these scripts copied at this stage?

Copy link
Member Author

Choose a reason for hiding this comment

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

no they weren't. i moved the copying to happen earlier

@@ -252,7 +268,7 @@ func provisionCluster(cmd *cobra.Command, args []string) (retErr error) {
if err != nil {
return err
}
err = _cmd(cli, nodeContainer(prefix, nodeName), "if [ -f /scripts/extra-pre-pull-images ]; then scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i vagrant.key -P 22 /scripts/extra-pre-pull-images [email protected]:/tmp/extra-pre-pull-images; fi", "copying /scripts/extra-pre-pull-images if existing")
err = _cmd(cli, nodeContainer(prefix, nodeName), "", "copying /scripts/extra-pre-pull-images if existing")
Copy link
Member

Choose a reason for hiding this comment

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

What do we do here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think was left erroneously from when i was testing stuff. i'll remove it

@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 11, 2024
Copy all the scripts directory to the vm during provisioning.
Modify all instances of ssh.sh to use the sshClient instead.
Expose host ports in provisioning by default

Signed-off-by: aerosouund <[email protected]>
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Sep 11, 2024
@kubevirt-bot
Copy link
Contributor

@aerosouund: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
check-up-kind-1.27-vgpu d5c3057 link false /test check-up-kind-1.27-vgpu
check-up-kind-1.30-vgpu d5c3057 link false /test check-up-kind-1.30-vgpu
check-provision-k8s-1.31 d5c3057 link true /test check-provision-k8s-1.31

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

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 8, 2024
@kubevirt-bot
Copy link
Contributor

PR needs rebase.

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants