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

Create a script to deploy minikube on a VM. #459

Merged
merged 4 commits into from
Mar 22, 2018

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Mar 20, 2018

  • This is the first step in adding E2E testing on minikube.
  • We add a function call to test_deploy that will deploy minikube on a VM.
  • The script copies the relevant kubeconfig information and certificates
    to a directory.
  • In a follow on PR we will incorporate this new command into our E2E
    workflow in order to create an E2E test that runs on minikube.
  • Related to minikube testing testing#6

This change is Reviewable

* This is the first step in adding E2E testing on minikube.
* We add a function call to test_deploy that will deploy minikube on a VM.
* Also provide a function to teardown the VM.
* The script copies the relevant kubeconfig information and certificates
  to a directory.
* In a follow on PR we will incorporate this new command into our E2E
  workflow in order to create an E2E test that runs on minikube.
* Related to kubeflow#6
@jlewi jlewi changed the title [WIP] Create a script to deploy minikube on a VM. Create a script to deploy minikube on a VM. Mar 20, 2018
@jlewi
Copy link
Contributor Author

jlewi commented Mar 20, 2018

/assign @lluunn

install_script = os.path.join(os.path.dirname(__file__), "install_minikube.sh")

if not os.path.exists(install_script):
logging.error("C %s", install_script)
Copy link
Contributor

Choose a reason for hiding this comment

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

incomplete message?

vm_util.execute_script(args.project, args.zone, args.vm_name, install_script)
vm_util.execute(args.project, args.zone, args.vm_name, ["sudo minikube start --vm-driver=none --disk-size=40g"])

# Copy the .kube and .minikube files to test_dir
Copy link
Contributor

Choose a reason for hiding this comment

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

why for loop for one item?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

if not os.path.exists(minikube_dir):
os.makedirs(minikube_dir)

for target in ["~/.minikube/*.crt", "~/.minikube/client.key"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move .kube here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -289,6 +298,122 @@ def ks_deploy(app_dir, component, params, env=None, account=None):
apply_command.append("--as=" + account)
util.run(apply_command, cwd=app_dir)

def modify_minikube_config(config_path, certs_dir):
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the purpose of doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to make the config work inside the pod and not the VM where minikube is running.
Added a comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@jlewi
Copy link
Contributor Author

jlewi commented Mar 22, 2018

@lluunn PTAL.

@@ -289,6 +298,122 @@ def ks_deploy(app_dir, component, params, env=None, account=None):
apply_command.append("--as=" + account)
util.run(apply_command, cwd=app_dir)

def modify_minikube_config(config_path, certs_dir):
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@lluunn
Copy link
Contributor

lluunn commented Mar 22, 2018

/lgtm
Please fix the conflict

@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 22, 2018
@jlewi
Copy link
Contributor Author

jlewi commented Mar 22, 2018

@lluunn Can you re LGTM?

@lluunn
Copy link
Contributor

lluunn commented Mar 22, 2018

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lluunn

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 merged commit febb21d into kubeflow:master Mar 22, 2018
elenzio9 pushed a commit to arrikto/kubeflow that referenced this pull request Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants