Verify cleanup of odh-deployer resources.

[VaishnaviHire]

This is a tracker issue to ensure there are no additional resources like duplicate secrets/configmaps as part of upgrade

odh-deployer repo: https://github.com/red-hat-data-services/rhods-operator

[VaishnaviHire]

This is a subtask of #461. Not adding qe-verification label, since it will be tested as part of upgrade.

[lugi0]

I see a problem with this, there are duplicate configmaps for DSPO-config and DSPO-parameters

[diegolovison]

cc @HumairAK

[HumairAK]

Some context: These configmaps are created using the configmap generator via kustomize, they are treated as immutable (though without explicitly setting them so), each new kustomize build will generate a new configmap which gets mounted on DSPO to trigger a new deployment (just updating the existing configmap does not trigger new deployments).

Not sure what the proper process to help with clean up here ought to be. How is ODH V2 Operator cleaning up resources? Would adding labels here help?

[lugi0]

AFAIK the controller of the component is responsible for creating / cleaning up its own resources, but let me ping @zdtsw @VaishnaviHire for a proper answer

[VaishnaviHire]

Hi @lugi0 @HumairAK Wouldn't this be an issue with v1.x upgrades as well?

This is correct, operator would not be able to add owner references to the resources it doesn't create. So component controller needs to manage the generated resources.

AFAIK the controller of the component is responsible for creating / cleaning up its own resources, but let me ping @zdtsw @VaishnaviHire for a proper answer

[VaishnaviHire]

@HumairAK Alternatively, manifests can be moved to kustomize v5 where params configmap is not generated

[zdtsw]

i was thinking about this, wont other component which uses configmap generator have the same problem?

[VaishnaviHire]

Yeah, however this is not introduced by the upgrade path and would be fixed when #714 is completed

[HumairAK]

AFAIK the controller of the component is responsible for creating / cleaning up its own resources
operator would not be able to add owner references to the resources it doesn't create

These resources are created when the dspo is deployed, it is my understanding the ODH operator does a kustomize build behind the scene, and applies the manifests, so it's ODH that creates these manifests.

@HumairAK Wouldn't this be an issue with v1.x upgrades as well?

Yes

@HumairAK Alternatively, manifests can be moved to kustomize v5 where params configmap is not generated

Not sure I follow, do you have a reference link? I haven't looked into v5 much.

We can just make these configmap names not generated with a hash suffix, but any changes to the configmap would require a manual restart of the deployment, it's not ideal, but if this is the formula the rest of the components are following, we can follow suite.

[lugi0]

@VaishnaviHire @zdtsw @HumairAK is this planned to be fixed inside RC2? or are we moving it to a future release?

[zdtsw]

@VaishnaviHire @zdtsw @HumairAK is this planned to be fixed inside RC2? or are we moving it to a future release?

dont think we had logic updated from operator side for getting this in RC2

[VaishnaviHire]

@lugi0 This is not included in RC2. It will be fixed as part of #714 . As Humair confirmed, this issue is not introduced in v1-->v2 upgrade

@HumairAK Kustomize v5 does not support param.env kubeflow/manifests#538

[hbelmiro]

@VaishnaviHire I'm not sure I'm following.
DSPO is already using the new syntax: https://github.com/opendatahub-io/data-science-pipelines-operator/blob/5dbf01264bd479591d2dfca89007b005f787b4d1/config/base/kustomization.yaml#L15C7-L15C7

cc @HumairAK

[VaishnaviHire]

Moving discussion to jira as part of epic for #714