Inherit namespace from ksonnet environment #426

ankushagarwal · 2018-03-14T19:07:45Z

Now that ksonnet makes env available in it's components, we can
leverate that to use the environment's namespace instead of hardcoding a
default namespace in the component parameters.

So now the behaviour is that when components are generated from
prototypes, we default to using the environment's namespace. If the user
has overridden namespace as a parameter, that will get used for the
component.

Tested by deploying kubeflow-core and tf-serving to a new k8s cluster

Fixes #43

This change is

ankushagarwal · 2018-03-14T19:08:52Z

/cc @lluunn

Now that ksonnet makes `env` available in it's components, we can leverate that to use the environment's namespace instead of hardcoding a default namespace in the component parameters. So now the behaviour is that when components are generated from prototypes, we default to using the environment's namespace. If the user has overridden namespace as a parameter, that will get used for the component. Tested by deploying kubeflow-core and tf-serving to a new k8s cluster Fixes #43

lluunn · 2018-03-14T22:57:40Z

With #411 test should pass

lluunn · 2018-03-15T00:43:01Z

kubeflow/core/jupyterhub.libsonnet

@@ -1,7 +1,4 @@
 {
-  // TODO(https://github.com/ksonnet/ksonnet/issues/222): Taking namespace as an argument is a work around for the fact that ksonnet
-  // doesn't support automatically piping in the namespace from the environment to prototypes.
-  //


Isn't this TODO for removing namespace param in parts(namespace)?

The comment in the TODO mentions that Taking namespace as an argument is a work around for the fact that ksonnet doesn't support automatically piping in the namespace from the environment to prototypes - I don't think that is true.

The workaround for ksonnet not piping the namespace from env to prototypes is we have an explicit parameter called namespace in our prototype - this PR addresses this issue by removing the namespace parameter from our prototypes.

lluunn · 2018-03-15T00:43:23Z

kubeflow/argo/prototypes/argo.jsonnet

-std.prune(k.core.v1.list.new(argo.parts(params.namespace).all))
+// updatedParams includes the namespace from env by default.
+// We can override namespace in params if needed
+local updatedParams = env + params;


env is the params from environment?
Aren't we already setting per environment param to override default?

env is not params. params are defined per component in component/params.libsonnet and can be overridden per-environment in environments/<env>/params.libsonnet

env is the ksonnet environment definition. It is the value you get when you execute ks env list

For example

% ks env list NAME KUBERNETES-VERSION NAMESPACE SERVER ==== ================== ========= ====== default v1.7.0 kubeflow https://35.184.102.122```

For this default environment, env will contain

{ "namespace": "kubeflow", "server": "https://35.184.102.122", }

The main idea is that we should not have to define the namespace parameter per component, it will automatically be inherited by the environment in which we do a ks apply <env>

ankushagarwal · 2018-03-15T01:28:41Z

/retest

lluunn · 2018-03-15T17:45:29Z

/retest

break existing clients

ankushagarwal · 2018-03-15T20:50:43Z

/retest

ankushagarwal · 2018-03-15T21:17:47Z

/retest

ankushagarwal · 2018-03-15T21:31:13Z

/retest

ankushagarwal · 2018-03-15T21:40:48Z

/retest

ankushagarwal · 2018-03-15T22:20:43Z

/retest

ankushagarwal · 2018-03-15T22:38:09Z

/retest

…ws.libsonnet

lluunn · 2018-03-15T23:21:45Z

For workflow kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359
I see
INFO|2018-03-15T23:08:56|/mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/py/util.py|43| Running: ks param set --env=test-env-3033 simple_tfjob namespace kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359
cwd=/mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/test/workflows
INFO|2018-03-15T23:08:57|/mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/py/util.py|82| Subprocess output:
Your application's apiVersion is below 0.1.0. In order to use all ks features, you can upgrade your application using ks upgrade.
Parameter 'namespace' successfully set to '"kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359"' for component 'simple_tfjob' in environment 'test-env-3033'

INFO|2018-03-15T23:08:57|/mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/py/test_runner.py|163| Trial 0
INFO|2018-03-15T23:08:57|/mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/py/util.py|43| Running: ks apply test-env-3033 -c simple_tfjob
cwd=/mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/test/workflows
INFO|2018-03-15T23:08:57|/mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/py/util.py|82| Subprocess output:
Your application's apiVersion is below 0.1.0. In order to use all ks features, you can upgrade your application using ks upgrade.
generate objects for namespace : unable to read /mnt/test-data-volume/kubeflow-presubmit-kubeflow-e2e-426-34cf6d3-644-1359/src/kubeflow/tf-operator/test/workflows/environments/test-env-3033/main.jsonnet: RUNTIME ERROR: Couldn't open import "k.libsonnet": No match locally or in the Jsonnet library paths.

lluunn · 2018-03-16T00:03:27Z

kubeflow/training-operator#464 should fix tf-operator test

ankushagarwal · 2018-03-16T02:11:28Z

/retest

jlewi · 2018-03-17T22:24:14Z

This is great thanks!

* Update jupyterhub.libsonnet comment * Set imagePullPolicy to always * Print ks version * Update tfserving test to use kubeflow-ci/test-worker

ankushagarwal · 2018-03-19T22:41:58Z

@jlewi Can you please take a look again. I pushed a few changes to fix tests.

jlewi · 2018-03-20T17:02:37Z

/lgtm
/approve

k8s-ci-robot · 2018-03-20T17:02:43Z

[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:

~~OWNERS~~ [jlewi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* Update docs so that namespace is only set at ks env level Now that #426 is merged, we do not need to set namespace at the component level. namespace will be automatically inherited by the component from env. * Update ksonnet version requirement

* Restore Metadata fix (kubeflow#424) * Fix metadata deployment for KFP's UI * Only update metadata test files * This PR was originally committed as kubeflow#424 but it got rolled back in kubeflow#429 in order to fix master. * This PR is rolling it forward. * Related to kubeflow#426 * Add the virtual service for grpc metadata to kustomization.yaml * Port for the virtual service should be 9090. * Fix tests.

k8s-ci-robot requested review from jimexist and zjj2wry March 14, 2018 19:07

k8s-ci-robot added the size/L label Mar 14, 2018

k8s-ci-robot requested a review from lluunn March 14, 2018 19:08

lluunn reviewed Mar 15, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into top-level-namespace

bcba163

lluunn mentioned this pull request Mar 15, 2018

Upgrade ksonnet from 0.8 to 0.9 for testing image kubeflow/testing#64

Merged

Ankush Agarwal added 2 commits March 15, 2018 12:54

Merge remote-tracking branch 'upstream/master' into top-level-namespace

1d2cc3f

Re-introduce namespace parameter in components because removing it will

fd046f4

break existing clients

Update mlkube-testing image to kubeflow-ci image

757b2ad

Ankush Agarwal added 2 commits March 15, 2018 15:43

Revert components/k8s-model-server/images/releaser/components/workflo…

dde6fd9

…ws.libsonnet

Update mlkube-testing to kubeflow-ci

34cf6d3

lluunn mentioned this pull request Mar 15, 2018

Presubmit shows succeeded, but some test actually failed. #436

Closed

Merge remote-tracking branch 'upstream/master' into top-level-namespace

f6a8b80

k8s-ci-robot removed the size/L label Mar 19, 2018

k8s-ci-robot added size/XXL size/L and removed size/XXL labels Mar 19, 2018

Undo changes to releasing files as they are unrelated

5dedb62

* Update jupyterhub.libsonnet comment * Set imagePullPolicy to always * Print ks version * Update tfserving test to use kubeflow-ci/test-worker

k8s-ci-robot assigned jlewi Mar 20, 2018

k8s-ci-robot added the lgtm label Mar 20, 2018

k8s-ci-robot added the approved label Mar 20, 2018

k8s-ci-robot merged commit 8d617d6 into kubeflow:master Mar 20, 2018

ankushagarwal deleted the top-level-namespace branch March 20, 2018 17:45

ankushagarwal mentioned this pull request Mar 20, 2018

Update docs so that namespace is only set at ks env level #463

Merged

elenzio9 pushed a commit to arrikto/kubeflow that referenced this pull request Oct 31, 2022

Include lynnmatrix as a member (kubeflow#426)

4d8b215

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inherit namespace from ksonnet environment #426

Inherit namespace from ksonnet environment #426

ankushagarwal commented Mar 14, 2018 •

edited by jlewi

ankushagarwal commented Mar 14, 2018

lluunn commented Mar 14, 2018

lluunn Mar 15, 2018

ankushagarwal Mar 15, 2018

lluunn Mar 15, 2018

ankushagarwal Mar 15, 2018

ankushagarwal commented Mar 15, 2018

lluunn commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

lluunn commented Mar 15, 2018 •

edited

lluunn commented Mar 16, 2018

ankushagarwal commented Mar 16, 2018

jlewi commented Mar 17, 2018

ankushagarwal commented Mar 19, 2018

jlewi commented Mar 20, 2018

k8s-ci-robot commented Mar 20, 2018

Inherit namespace from ksonnet environment #426

Inherit namespace from ksonnet environment #426

Conversation

ankushagarwal commented Mar 14, 2018 • edited by jlewi

ankushagarwal commented Mar 14, 2018

lluunn commented Mar 14, 2018

lluunn Mar 15, 2018

Choose a reason for hiding this comment

ankushagarwal Mar 15, 2018

Choose a reason for hiding this comment

lluunn Mar 15, 2018

Choose a reason for hiding this comment

ankushagarwal Mar 15, 2018

Choose a reason for hiding this comment

ankushagarwal commented Mar 15, 2018

lluunn commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

ankushagarwal commented Mar 15, 2018

lluunn commented Mar 15, 2018 • edited

lluunn commented Mar 16, 2018

ankushagarwal commented Mar 16, 2018

jlewi commented Mar 17, 2018

ankushagarwal commented Mar 19, 2018

jlewi commented Mar 20, 2018

k8s-ci-robot commented Mar 20, 2018

ankushagarwal commented Mar 14, 2018 •

edited by jlewi

lluunn commented Mar 15, 2018 •

edited