canonical / kubeflow-tensorboards-operator

Tensorboards Operator
Apache License 2.0
2 stars 5 forks source link

feat: Rewrite the Tensorboard Controller Charm following the sidecar pattern #69

Closed phoevos closed 1 year ago

phoevos commented 1 year ago

Rewrite the Tensorboard Controller Charm following the sidecar pattern. Summary of changes:

Upgrade Instructions

No special steps required, apart from trusting the tensorboard-controller application and providing the image on refresh since we changed the oci-image resource to tensorboard-controller-image:

  1. Trust the application
    juju trust tensorboard-controller
  2. Refresh the charm providing either the path to the local charm (e.g. --path ./tensorboard-controller_ubuntu-20.04-amd64.charm) or the channel if the desired release is published on Charmhub.
    juju refresh tensorboard-controller \
    --resource tensorboard-controller-image=kubeflownotebookswg/tensorboard-controller:v1.7.0
ca-scribner commented 1 year ago

@phoevos can you open (and link in description) a PR on kubeflow-roles-operator that removes the Roles that you migrated over here to the charm? Without that, we'll have some resource conflict issues on final bundle testing

ca-scribner commented 1 year ago

Have you tested upgrading from the previous version to this one? Please include some upgrade instructions here (could be as simple as juju refresh or could have additional steps. Could even be "you can't" :D )

ca-scribner commented 1 year ago

Oh, and we should document how port config was removed. That might actually mess with the upgrade procedures (not sure what a charm does if you upgrade and a config was removed). Good to test and document here

i-chvets commented 1 year ago

I would even go furthern and add upgrade test.

ca-scribner commented 1 year ago

You've added charm-level integration tests - we need to add those to the CI as well

phoevos commented 1 year ago

@phoevos can you open (and link in description) a PR on kubeflow-roles-operator that removes the Roles that you migrated over here to the charm? Without that, we'll have some resource conflict issues on final bundle testing

I don't see anything associated with the TB Controller there (while I do see the TWA-specific roles), am I missing something?

DnPlas commented 1 year ago

@phoevos can you open (and link in description) a PR on kubeflow-roles-operator that removes the Roles that you migrated over here to the charm? Without that, we'll have some resource conflict issues on final bundle testing

I don't see anything associated with the TB Controller there (while I do see the TWA-specific roles), am I missing something?

You are right @phoevos, the kubeflor-roles-operator does not handle any tensorboard-controller aggregation clusterRole. No further action is needed.