util.libsonnet has methods that allow a <component>.part to be modified

[kkasravi]

Fixes #1231

After kfctl.sh generate k8s the user would update components/jupterhub.jsonnet

local jupyterhub = import "kubeflow/core/jupyterhub.libsonnet";
local util = import "kubeflow/core/util.libsonnet";
util.update(jupyterhub, updatedParams, 'StatefulSet/tf-hub',
  {
    template+: {
      spec+: {
        imagePullSecrets: [
          {name: "mySecret",}
        ],
      },
    },
  }
)

---

This change is 

[kkasravi]

/assign @jlewi

[kkasravi]

/cc @ankushagarwal

[ankushagarwal]

/lgtm
/approve

[ankushagarwal]

/lgtm
/approve

[kkasravi]

/retest

[jlewi]

See comment here:
#1231 (comment)

It looks like the pattern you are suggesting is to have parts define a list and then convert that to a map so you can modify an element by name. That's a useful function.

But why wouldn't we just rewrite the libsonnet file to be a map?

e.g suppose the libsonnet file looked like

{
    statefulset :: {
     ...
   }

   parts :: [$.statefulset, ....]
}

Then I think in the .jsonnet file you could do something like

local jupyterhub = import "kubeflow/core/jupyterhub.libsonnet";
local updated = jupterhub +  {
   statfulset +: {
        template+: {
      spec+: {
        imagePullSecrets: [
          {name: "mySecret",}
        ],
      },
    },
  }
 },
)

std.prune(updated.parts)

[jlewi]

Why is toMap taking params as an argument?

[kkasravi]

I believe I understand. Does the following make sense?

1. Remove params from toMap (prometheus.libsonnet is an example where parts is a map and all is parts)
2. Build the map first, then the parts which reference the map eg: [map.values()]

Question: What about repeating resources within a libsonnet? For example jupyterhub.libsonnet has 2 Services, 2 ServiceAccounts, 2 Roles, 2 RoleBindings. Should we deal with it only if needed? For example:

local jup = {
  Service: {
    "tf-hub-0": {foo:"bar",},
    "tf-hub-lb": {bar:"baz",},
  },
};

so the update would look like

jup +  {
   Service+: {
    "tf-hub-0"+: {one: "two",},
  },
}

yielding

{
   "Service": {
      "tf-hub-0": {
         "foo": "bar",
         "one": "two"
      },
      "tf-hub-lb": {
         "bar": "baz"
      }
   }
}

for non-repeating resources it would look like your example with StatefulSet or would that be too confusing?

[jlewi]

I don't think the top level items in the dictionary should be based on type. They should be based on what they do; e.g. if there are multiple services each service should be its own item.

[kkasravi]

so how bout

{
  ConfigMap:: 
  HubService::
  HubServiceAccount::
  HubRole::
  HubRoleBinding::
  NotebookService::
  NotebookServiceAccount::
  NotebookRole::
  NotebookRoleBinding::  
  StatefulSet::
}

where the map is a set of functional hidden names. The one above would be jupyterhub.

[kkasravi]

parts (or all) would be in the top level mapping

{
  ConfigMap:: 
  HubService::
  HubServiceAccount::
  HubRole::
  HubRoleBinding::
  NotebookService::
  NotebookServiceAccount::
  NotebookRole::
  NotebookRoleBinding::  
  StatefulSet::

  parts|all:: [ ConfigMap:: HubService:: ... ]
}

[jlewi]

LGTM

[kkasravi]

@jlewi

This now works

local jupyterhub = import "kubeflow/core/jupyterhub.libsonnet";
local updated = jupyterhub + {
  StatefulSet+: {
     spec+: {
       template+: {
         spec+: {
          imagePullSecrets: [
            {name: "mySecret",}
          ],
        },
      },
    },
  },
};
updated.all(updatedParams).list

I can update the other libsonnets if this looks ok. Note that params is now referenced as $.params within the resources.

[jlewi]

nit: Change the name and StatefulSet name to be more descriptive
e.g
ConfigMap -> KubeSpawnerConfig

StatefulSet -> HubStatefulSet

[jlewi]

Looks great to me. I had one nit about the names but otherwise this is ready IMO.

[kkasravi]

thanks @jlewi

[jlewi]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ankushagarwal, 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 [ankushagarwal,jlewi]

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