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

Replace 'cloud' parameter with more direct names like 'params.tfDefaultImage' and 'params.platform' #1742

Merged
merged 10 commits into from Oct 20, 2018

Conversation

ashahba
Copy link
Member

@ashahba ashahba commented Oct 10, 2018

fixes #1227


This change is Reviewable

@ashahba
Copy link
Member Author

ashahba commented Oct 10, 2018

/assign @jlewi
I'm getting a bunch of iam errors during runs.

 O='ERROR: (gcloud.iam.service-accounts.describe) PERMISSION_DENIED: Permission iam.serviceAccounts.get is required to perform this operation on service account projects/-/serviceAccount
s/kfctl-8767-admin@kubeflow-ci.iam.gserviceaccount.com.'
+ local RESULT=1
+ '[' 1 -ne 0 ']'
+ echo Service account kfctl-8767-admin@kubeflow-ci.iam.gserviceaccount.com 'doesn'\''t' exist or you do not have permission to access service accounts.
+ return
+ for suffix in '"${accounts[@]}"'
+ SA=kfctl-8767-user@kubeflow-ci.iam.gserviceaccount.com
+ deleteSa kfctl-8767-user@kubeflow-ci.iam.gserviceaccount.com
+ local SA=kfctl-8767-user@kubeflow-ci.iam.gserviceaccount.com
Service account kfctl-8767-admin@kubeflow-ci.iam.gserviceaccount.com doesn't exist or you do not have permission to access service accounts.
++ gcloud --project=kubeflow-ci iam service-accounts describe kfctl-8767-user@kubeflow-ci.iam.gserviceaccount.com

I believe I'm missing some permissions to run presubmit tests.
Would you please double check?

Thanks.

@ashahba
Copy link
Member Author

ashahba commented Oct 10, 2018

/retest

@jlewi
Copy link
Contributor

jlewi commented Oct 12, 2018

Thanks!

Any chance you could add tests for these modules like these
https://github.com/kubeflow/kubeflow/tree/master/kubeflow/core/tests

@jlewi
Copy link
Contributor

jlewi commented Oct 14, 2018

I think you need to pull in the latest changes on master and rebase in order to fix the tests.

@jlewi
Copy link
Contributor

jlewi commented Oct 15, 2018

Here are the logs from the failed test

E  + ks apply default -c ambassador
 
E  level=error msg="find objects: RUNTIME ERROR: Field does not exist: platform\n\t/mnt/test-data-volume/kubeflow-presubmit-kfctl-1742-15370c5-3820-79aa/kfctl-79aa/ks_app/vendor/kubeflow/core/ambassador.libsonnet:149:27-42\tobject <anonymous>\n\t<std>:1165:25-26\tthunk from <thunk <ta> from <function <anonymous>>>\n\t<builtin>\tbuiltin function <type>\n\t<std>:1167:29-31\tthunk from <function <anonymous>>\n\t<builtin>\tbuiltin function <primitiveEquals>\n\t<std>:1167:8-36\tfunction <anonymous>\n\t\t\n\t/mnt/test-data-volume/kubeflow-presubmit-kfctl-1742-15370c5-3820-79aa/kfctl-79aa/ks_app/vendor/kubeflow/core/ambassador.libsonnet:157:19-27\tobject <anonymous>\n\tDuring manifestation\t\n"
 
  undefined

So it looks like there is a problem with the ksonnet.

You should be able to debug locally just by creating a test app and running "ks show"

@jlewi
Copy link
Contributor

jlewi commented Oct 15, 2018

Even better would be to add a unittest for the ambassador prototype here
https://github.com/kubeflow/kubeflow/tree/master/kubeflow/core/tests

@ashahba
Copy link
Member Author

ashahba commented Oct 19, 2018

@jlewi this is done.
But still need to write test cases for ambassador.
We can address the testing here or in a separate ticket.
Let me know what you think.

Thanks.

@jlewi
Copy link
Contributor

jlewi commented Oct 20, 2018

Separate PR SGTM

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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 e41ace2 into kubeflow:master Oct 20, 2018
@ashahba ashahba deleted the ashahba/fix-1227 branch February 10, 2019 23:36
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
…ltImage' and 'params.platform' (kubeflow#1742)

* Replace 'cloud' parameter with more direct names like 'params.tfDefaultImage' and 'platform'

* Fix an style error and also renamed a duplicate param within tf-serving.libsonnet

* Fix an style failure and also renamed a duplicate param within tf-serving.libsonnet

* Fix a type in storageType

* Fix a failing test for jupyterhub

* Fix missing ks param platform in kfctl.sh

* Rename CLOUD_NAME to PLATFORM_NAME
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.

Remove ksonnet parameter cloud
4 participants