fix commonLabels

[thesuperzapper]

Description of your changes:
Makes the following components have consistent commonLabels between base and base_v3:

• centraldashboard
• jupyter-web-app
• notebook-controller

Checklist:

• Unit tests have been rebuilt:
  1. cd manifests/tests
  2. make generate-changed-only
  3. make test

[thesuperzapper]

@Jeffwan can you please review?

[Jeffwan]

It looks good to me. I am not sure if there's still user on base/ probably some 1.0 users. In the future, we may want to remove and deprecate legacy manifests

/lgtm

[Jeffwan]

/approve

[thesuperzapper]

@PatrickXYS can you approve this, as we need someone who is in the root OWNERS file

[thesuperzapper]

@Bobgy could you also approve this?

[Bobgy]

Generally updating labels is a breaking change to upgrade experience.
We can document to tell users delete old deployments when they see the label selector immutable error message, but it's not ideal.

So I'd want to make sure we should minimize such changes as much as possible in the future.

[Bobgy]

Looking at your PR, this label shouldn't exist now. Is this a mistake?

[thesuperzapper]

That will be coming from the kustomization.yaml in overlay/application: https://github.com/kubeflow/manifests/blob/master/common/centraldashboard/overlays/application/kustomization.yaml

[Bobgy]

Sorry, still not fully clear.
Why do some test snapshots have the two labels, while some do not?
Is that expected?

[thesuperzapper]

If the stack is using the application overlay, there will be both, but not otherwise (because I removed them from the base).

Ones from application overlay:

    app.kubernetes.io/component: centraldashboard
    app.kubernetes.io/name: centraldashboard

Ones from base:

    app: centraldashboard
    kustomize.component: centraldashboard

[Bobgy]

Thanks for clarification! Created an issue to track the problem: #1649.

[Bobgy]

/cc @PatrickXYS
the tests didn't fail, so I guess we may have lost tests for #1131 or the tests were not added properly initially?

[thesuperzapper]

@Bobgy I agree that we need to confirm if we want to force users to remove their Deployment resources during the next upgrade.

NOTE: I made these changes to make the labels consistent.

[Bobgy]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy, Jeffwan, thesuperzapper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [Bobgy]

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