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

Improvements and bug fixes in DM config. #904

Merged
merged 3 commits into from Jun 1, 2018
Merged

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Jun 1, 2018

  • Create a service account to be used to authorize TFJobs and other work
    within the cluster.

  • Create a helper script to download service account keys and turn
    them into K8s keys

  • Fix some bugs in the docs.

  • Fix Deployment manager config should create service accounts and set IAM roles #878 create a GCP service account for the user.

  • IAP script needs a GCP service account with network admin privileges.

  • Add network admin privileges to the admin service account.

  • Name the secrets in K8s so that be default the names are the same across
    the deployments. This way there's one less parameter to set for
    every deployment.

  • VM service account should have a unique name per deployment so deployments
    are isolated.

  • Need to grant the VM service account logs and monitoring access to support
    monitoring.

  • I don't think there's any reason to allow user to specify name of the
    VM service account in the YAML file right now.


This change is Reviewable

* Create a service account to be used to authorize TFJobs and other work
  within the cluster.

* Create a helper script to download service account keys and turn
  them into K8s keys

* Fix some bugs in the docs.

* Fix kubeflow#878 create a GCP service account for the user.

* IAP script needs a GCP service account with network admin privileges.
* Add network admin privileges to the admin service account.
* Name the secrets in K8s so that be default the names are the same across
  the deployments. This way there's one less parameter to set for
  every deployment.

* VM service account should have a unique name per deployment so deployments
  are isolated.

* Need to grant the VM service account logs and monitoring access to support
  monitoring.

* I don't think there's any reason to allow user to specify name of the
  VM service account in the YAML file right now.
@jlewi
Copy link
Contributor Author

jlewi commented Jun 1, 2018

/assign @ankushagarwal


# TODO(jlewi): We should name the secrets more consistently based on the service account name.
# We will need to update the component configs though
gcloud --project=${PROJECT} iam service-accounts keys create ${SA_EMAIL}.json --iam-account $SA_EMAIL
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use ${SA_EMAIL}

@@ -286,9 +300,40 @@ TODO(jlewi): Do we need to serialize API activation
members:
- {{ 'serviceAccount:' + env['project_number'] + '@cloudservices.gserviceaccount.com' }}

{# servicemanagement.admin is needed by CloudEndpoints controller
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we manage deletion of IAM Role Bindings? When we delete the deployment, the service accounts go away but these IAM Role Bindings still lurk around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Created
#910

@ankushagarwal
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ankushagarwal

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 removed the lgtm label Jun 1, 2018
@ankushagarwal
Copy link
Contributor

/lgtm

@jlewi
Copy link
Contributor Author

jlewi commented Jun 1, 2018

/test all

@k8s-ci-robot k8s-ci-robot merged commit 305d683 into kubeflow:master Jun 1, 2018
wmuizelaar pushed a commit to bolcom/kubeflow that referenced this pull request Jun 5, 2018
* Improvements and bug fixes in DM config.

* Create a service account to be used to authorize TFJobs and other work
  within the cluster.

* Create a helper script to download service account keys and turn
  them into K8s keys

* Fix some bugs in the docs.

* Fix kubeflow#878 create a GCP service account for the user.

* IAP script needs a GCP service account with network admin privileges.
* Add network admin privileges to the admin service account.
* Name the secrets in K8s so that be default the names are the same across
  the deployments. This way there's one less parameter to set for
  every deployment.

* VM service account should have a unique name per deployment so deployments
  are isolated.

* Need to grant the VM service account logs and monitoring access to support
  monitoring.

* I don't think there's any reason to allow user to specify name of the
  VM service account in the YAML file right now.

* Address comments.

* Autoformat jsonnet.
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
* Improvements and bug fixes in DM config.

* Create a service account to be used to authorize TFJobs and other work
  within the cluster.

* Create a helper script to download service account keys and turn
  them into K8s keys

* Fix some bugs in the docs.

* Fix kubeflow#878 create a GCP service account for the user.

* IAP script needs a GCP service account with network admin privileges.
* Add network admin privileges to the admin service account.
* Name the secrets in K8s so that be default the names are the same across
  the deployments. This way there's one less parameter to set for
  every deployment.

* VM service account should have a unique name per deployment so deployments
  are isolated.

* Need to grant the VM service account logs and monitoring access to support
  monitoring.

* I don't think there's any reason to allow user to specify name of the
  VM service account in the YAML file right now.

* Address comments.

* Autoformat jsonnet.
yanniszark pushed a commit to arrikto/kubeflow that referenced this pull request Feb 15, 2021
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.

None yet

3 participants