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

Enabling EFS on AWS for Tensorboard #2569

Merged
merged 2 commits into from
Mar 11, 2019

Conversation

jeremievallee
Copy link
Contributor

@jeremievallee jeremievallee commented Feb 27, 2019

Allowing the use of AWS EFS for the Tensorboard deployment on AWS.
Previously, only S3 parameters were available.

With these changes, provided you have a Persistent Volume Claim with an EFS resource on your cluster (using efs-provisioner), you can make Tensorboard read from EFS.

You can still use S3, of course. They are not mutually exclusive (use the flags efsEnabled and s3Enabled, both can be true at the same time).


This change is Reviewable

@k8s-ci-robot
Copy link
Contributor

Hi @jeremievallee. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@pdmack
Copy link
Member

pdmack commented Feb 27, 2019

/ok-to-test

@jeremievallee
Copy link
Contributor Author

/retest

1 similar comment
@jeremievallee
Copy link
Contributor Author

/retest

@jeremievallee jeremievallee force-pushed the efs-aws-tensorboard branch 2 times, most recently from 4ba5bc6 to c27cb0d Compare March 1, 2019 10:19
@jeremievallee
Copy link
Contributor Author

/assign @pdmack

@@ -8,13 +8,18 @@
// @optionalParam servicePort number 9000 Name of the servicePort
// @optionalParam serviceType string ClusterIP The service type for tensorboard service
// @optionalParam defaultTbImage string tensorflow/tensorflow:1.8.0 default tensorboard image to use
// @optionalParam s3Enabled string false Wether or not to use S3
Copy link
Member

Choose a reason for hiding this comment

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

nit: "Whether"

// @optionalParam s3SecretName string null Name of the k8s secrets containing S3 credentials
// @optionalParam s3SecretAccesskeyidKeyName string null Name of the key in the k8s secret containing AWS_ACCESS_KEY_ID
// @optionalParam s3SecretSecretaccesskeyKeyName string null Name of the key in the k8s secret containing AWS_SECRET_ACCESS_KEY
// @optionalParam s3AwsRegion string us-west-1 S3 region
// @optionalParam s3UseHttps string true Whether or not to use https
// @optionalParam s3VerifySsl string true Whether or not to verify https certificates for S3 connections
// @optionalParam s3Endpoint string s3.us-west-1.amazonaws.com URL for your s3-compatible endpoint
// @optionalParam efsEnabled string false Wether or not to use EFS
Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
Member

@pdmack pdmack left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pdmack

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

@pdmack
Copy link
Member

pdmack commented Mar 11, 2019

gah

INFO|2019-03-11T17:28:50|/src/kubeflow/testing/py/kubeflow/testing/util.py|71| level=info msg="Creating environment \"kubeflow-presubmit-kfctl-2569-c27cb0d-6224-e533\" with namespace \"kubeflow-test-infra\", pointing to \"version:v1.11.7\" cluster at address \"https://35.196.213.148\""
INFO|2019-03-11T17:29:00|/src/kubeflow/testing/py/kubeflow/testing/util.py|71| level=error msg="Get https://raw.githubusercontent.com/kubernetes/kubernetes/v1.11.7/api/openapi-spec/swagger.json: net/http: request canceled (Client.Timeout exceeded while awaiting headers)"

@pdmack
Copy link
Member

pdmack commented Mar 11, 2019

/retest

@pdmack
Copy link
Member

pdmack commented Mar 11, 2019

Sorry @jeremievallee but I think you need another rebase to pick up #2564

@jeremievallee
Copy link
Contributor Author

@pdmack yep, I think so too. I'll do that now. Thanks!

@pdmack
Copy link
Member

pdmack commented Mar 11, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit 06893f0 into kubeflow:master Mar 11, 2019
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
* Enabling EFS on AWS for Tensorboard

* Fixed comment typos.
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 12, 2021
* Enabling EFS on AWS for Tensorboard

* Fixed comment typos.
This pull request was closed.
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.

4 participants