Metrics-server should not create or addopt kube-system namespace.

[adduarte]

If the metric server creates or addopts the kube-system namespace,
it will be marked for deletion whenever the metric-server package is
deleted. kube-system namespace deletion is not allowed.

Fixes vmware-tanzu/tanzu-framework#1708

Describe testing done for PR

Special notes for your reviewer

[shyaamsn]

Couple of scenarios we need to handle:

1. User giving createNamespace: true and namespace: kube-system - In this case lets not create namespace
2. Upgrade: User upgrades the package but has older data values i.e. User in their data values has createNamespace: true. So even though package defaults are changed the user has createNamespace: true and hence during delete it will attempt to delete the namespace. Lets also add https://carvel.dev/kapp/docs/v0.46.0/apply/#kappk14siodelete-strategy delete-strategy:orphan in the namespace to prevent deletion of namespace

[adduarte]

namespace creation is handled by kapp-controller. I don't think we can do much here to prevent the creation or adoption of the namespace when the user intentionally sets createNamespace: true.
However adding delete-strategy:orphan to the namespace will mitigate this as well.

[adduarte]

It looks like this behavior has been fixed in carvel-dev/kapp#468

see discussion here: https://kubernetes.slack.com/archives/C02GY94A8KT/p1645018465370589
There is a request to add a feature to kapp to prevent a package from taking ownership: carvel-dev/kapp-controller#523

and the fix is referenced there.
looks like fix should be available as of kapp v0.47.0

[adduarte]

for deployments that use older kapp-controller the best approach will be to add https://carvel.dev/kapp/docs/v0.46.0/apply/#kappk14siodelete-strategy delete-strategy:orphan, and modify the values.yaml as suggested in this pr

[shyaamsn]

1. Lets add https://carvel.dev/kapp/docs/v0.46.0/apply/#kappk14siodelete-strategy delete-strategy:orphan for older kapp-controller deployments
2. Can we add a check here https://github.com/vmware-tanzu/community-edition/blob/main/addons/packages/metrics-server/0.5.1/bundle/config/overlays/overlay-namespace.yaml#L12
   #@ if data.values.metricsServer.createNamespace and data.values.namespace != "kube-system":
   To prevent even creating namespace if kube-system is given ?

[adduarte]

sounds like a good plan

[adduarte]

added check of != "kube-system" to 0.5.1,
added orphaned label to kube-system namespace on 0.4.1,
Not sure if this is the correct way to add the orphan label.

[adduarte]

The latest commit only sets createNamespace: false.
Since the change will be included weather or not we use the delete-strategy:orphan label, it is a good place to start.
We can add other changes as follow up prs if we find it is not enough

[adduarte]

In order to move forward we will scale down this patch to only fix the incorrect settings in metrics-server package bundle.
A different patch in tanz-framwork/addons/a will be used to correct any deployments which have the kube-system namespace marked with the kapp-controller annotations.

[adduarte]

selected this as a better solutionhttps://github.com//pull/4724