Manifest readiness for Operator v2

[VedantMahabaleshwarkar]

Description

• structured manfests such that the new operator can fetch the manifests from config/.
• made it easier to maintain sync with upstream by keeping all manifest overrides in overlays/odh so as to minimize merge conflicts while syncing with upstream.
• separated out modelmesh manifests from odh-model-controller manifests to enable the opendatahub-operator fetching manifests from each individual repo

Testing

The testing for this PR is to be done along with the testing for this and this PR since they target the same set of overall changes.
The ODH operator image has been built with the following changes :

• odh-model-controller manifests are pre built from https://github.com/VedantMahabaleshwarkar/odh-model-controller/tree/manifests_transition_test which has equivalent changes to this repo, except that it deploys the image quay.io/vedantm/odh-model-controller:remove-kserve-flag which contains the changes for this PR.
• model-mesh manifests are pre built from https://github.com/VedantMahabaleshwarkar/modelmesh-serving/tree/manifests_overlay and the manfest location is config/overlays/odh. These manifests are now no longer bundled with odh-model-controller manifests
• opendatahub-operator reconciliation for kserve and modelmesh allow a common deployment of odh-model-controller

How Has This Been Tested?

ModelMesh testing

• cluster login
• git clone --branch transition [email protected]:VedantMahabaleshwarkar/opendatahub-operator.git
• cd opendatahub-operator
• make deploy -e IMG=quay.io/vedantm/opendatahub-operator:version_builtin
• This will install the ODH operator in NS opendatahub-operator-system with the image quay.io/vedantm/opendatahub-operator:version_builtin
• create the following DSC in NS opendatahub

cat <<EOF |oc create -f - 
apiVersion: datasciencecluster.opendatahub.io/v1
kind: DataScienceCluster
metadata:
  name: example
  namespace: opendatahub
spec:
  components:
    modelmeshserving:
      managementState: Managed
EOF

• deploy and test a model with modelmesh

Kserve testing

Note: Cleanup kserve CRDs before testing modelmesh

• cluster login
• install ODH Kserve + dependencies stack by following instructions

Note: Only install the dependencies, the ODH operator will be a custom install according to next steps

• git clone [email protected]:VedantMahabaleshwarkar/opendatahub-operator.git
• cd opendatahub-operator
• switch to branch manifests_transition_mlserving
• make deploy -e IMG=quay.io/vedantm/opendatahub-operator:version_builtin
• This will install the ODH operator in NS opendatahub-operator-system with the image quay.io/vedantm/opendatahub-operator:version_builtin
• create the following DSC in NS opendatahub

apiVersion: datasciencecluster.opendatahub.io/v1
kind: DataScienceCluster
metadata:
  name: default
spec:
  components:
    kserve:
      managementState: Managed

• deploy and test a kserve model using instructions

Merge criteria:

• The commits are squashed in a cohesive manner and have meaningful messages.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: VedantMahabaleshwarkar

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 [VedantMahabaleshwarkar]

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

[Jooho]

@VedantMahabaleshwarkar do we need to add these two lines to deploy modelmesh?

  components:
    modelmeshserving:
      managementState: Managed
      devFlags:
        manifests:
          - manifestsFolder: config/overlays/odh
            uri: https://github.com/VedantMahabaleshwarkar/modelmesh-serving/tarball/manifests_overlay
          - manifestsFolder: config
            uri: https://github.com/VedantMahabaleshwarkar/modelmesh-serving/tarball/manifests_overlay

[VedantMahabaleshwarkar]

Custom ODH operator image was built with the following settings in get_all_manifests.sh for the operator :

["odh-model-controller"]="VedantMahabaleshwarkar:odh-model-controller:manifests_transition_test:config:odh-model-controller"
["model-mesh"]="VedantMahabaleshwarkar:modelmesh-serving:manifests_overlay:config:model-mesh"

[israel-hdez]

Custom ODH operator image was built with the following settings in get_all_manifests.sh for the operator :

["odh-model-controller"]="VedantMahabaleshwarkar:odh-model-controller:manifests_transition_test:config:odh-model-controller"
["model-mesh"]="VedantMahabaleshwarkar:modelmesh-serving:manifests_overlay:config:model-mesh"

I echo Jooho's comment.
Isn't it possible to test using devFlags in the operator? Or is there a specific reason for using your build?

By using your build, I feel like I'm not testing this PR, but potentially something completely different.

[israel-hdez]

In general and when comparing with results from odh-manifests repo, I'm see a different set of metadata.labels. Is this expected?

Also, nodeAffinity was removed from the controller. Is this expected?

• made it easier to maintain sync with upstream by keeping all manifest overrides in overlays/odh so as to minimize merge conflicts while syncing with upstream.

I'm not fully in favor of copying files. It will reduce conflicts, but it will also reduce awareness of customizations when syncing code. If we go forward with this way, please make sure all devs are aware that when doing code syncs upstream changes to config/ must be checked against our overrides.

[VedantMahabaleshwarkar]

I echo Jooho's comment.
Isn't it possible to test using devFlags in the operator? Or is there a specific reason for using your build?
By using your build, I feel like I'm not testing this PR, but potentially something completely different.

@israel-hdez not possible to test without changes to the operator, which are in the operator PR. Discussed this with the operator team today as well and because of the nature of the changes, these cannot be tested with devFlags

[VedantMahabaleshwarkar]

In general and when comparing with results from odh-manifests repo, I'm see a different set of metadata.labels. Is this expected?

Earlier, somehow the metadatalabeltransformer was only applied on some sections of the manifests and not others, not sure why that was. Now is is uniform and all the generated resources will have consistent labels. So yes the few extra labels you might see in a comparison are expected

[Jooho]

Modelmesh case

• I can deploy odh-model-controller and modelmesh-controller
  odh-model-controller: quay.io/vedantm/odh-model-controller:remove-kserve-flag
  modelmesh-controller: quay.io/opendatahub/modelmesh-controller:fast
  ==> Could you please explain why quay.io/vedantm/odh-model-controller:remove-kserve-flag is used?
  ==> Do we need to update PR to change this image tag?
• The replica of modelmesh-controller is just 1.
  • Have you differed between original manifests and new manifests?
• sample model is deployed and works fine
• when I changed the Managed to Removed, odh-model-controller was not uninstalled. @VaishnaviHire I think this would be operator issue. could you please take a look at this?

KServe case

• I can deploy odh-model-controller and modelmesh-controller
  odh-model-controller: quay.io/vedantm/odh-model-controller:remove-kserve-flag
  kserve-controller: quay.io/opendatahub/kserve-controller:v0.11.0
  ==> The same question with modelmesh case "Could you please explain why quay.io/vedantm/odh-model-controller:remove-kserve-flag is used?"
  ==> Do we need to update PR to change this image tag?
• sample model is deployed and works fine
• Same issue - when I changed the Managed to Removed, odh-model-controller was not uninstalled. @VaishnaviHire I think this would be operator issue. could you please take a look at this?

[Jooho]

Earlier, somehow the metadatalabeltransformer was only applied on some sections of the manifests and not others, not sure why that was. Now is is uniform and all the generated resources will have consistent labels. So yes the few extra labels you might see in a comparison are expected

No, we must make it the same. The label difference can stop the upgrade

[VedantMahabaleshwarkar]

@Jooho

"Could you please explain why quay.io/vedantm/odh-model-controller:remove-kserve-flag is used?"

quay.io/vedantm/odh-model-controller:remove-kserve-flag this image contains the code changes from opendatahub-io/odh-model-controller#92

[israel-hdez]

not possible to test without changes to the operator, which are in the operator PR.

OK, I will do a review of all together. What's the operator PR? The one in the description is closed.

[VedantMahabaleshwarkar]

@israel-hdez

What's the operator PR?

opendatahub-io/opendatahub-operator#639

[VedantMahabaleshwarkar]

@israel-hdez corrected the label mismatch

[israel-hdez]

Similarly to the odh-model-controller PR, there is a ClusterRoleBinding that is being renamed here:

modelmesh-controller-rolebinding vs modelmesh-controller-rolebinding-opendatahub

[openshift-ci]

@VedantMahabaleshwarkar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/fvt-odh-manifests	73b2399	link	true	/test fvt-odh-manifests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[Jooho]

fvt failed because of transition manifests so I manually merged and update images

• quay.io/opendatahub/modelmesh-controller:fast-73b2399
• quay.io/opendatahub/modelmesh-controller:fast