kserve: configure servicemesh before deploying manifests

[ykaliuta]

Jira: https://issues.redhat.com/browse/RHOAIENG-7312

kserve depends on odh-model-controller which it starts by deploying
manifests of Dependent Operator. The controller behaviour depends of
configuration (authorino) which is later deployed by configuring
servicemesh features. Here is a race, there are 2 checks in the
odh-model-controller for the presence of AuthorizationPolicy (which
is deployed by servicemesh configuration):

1. to add a type to the schema
2. to watch the objects of that type.

If the object appears in between odh-model-controller complains:

2024-05-16T06:46:03Z ERROR Reconciler error {"controller": "inferenceservice", "controllerGroup": "serving.kserve.io", "controllerKind": "InferenceService", "InferenceService": {"name":"xf","namespace":"single-model-test"}, "namespace": "single-model-test", "name": "xf", "reconcileID": "e6f42f44-1866-45d4-836a-69e4e93edef4", "error": "1 error occurred:\n\t* could not GET authconfig single-model-test/xf. cause no kind is registered for the type v1beta2.AuthConfig in scheme \"pkg/runtime/scheme.go:100\"\n\n"}   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler    /remote-source/deps/gomod/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:329   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem    /remote-source/deps/gomod/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:274   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
 /remote-source/deps/gomod/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:235

Move servicemesh configuration before deploying the manifests to
narrow the race window.

odh-model-controller will change their check in favor of checking
Authorino CRD to avoid the race completely.

Checking AuthorizationPolicy existance in operator/kserve would fix
it as well, but it's a delay for the reconcile loop (vs
odh-model-controller where it's done only on startup), so since
odh-model-controller is going to reimplement the check, keep it as
it is.

Also modelmeshserving component can deploy
odh-model-controller (thanks Vedant for pointing) if it is
enabled. The order is unspecified but due to implementation it will
happen before kserve configuration (order of field in the Components
structure).

Signed-off-by: Yauheni Kaliuta [email protected]

How Has This Been Tested?

• deploy operator
• create DSCI and DSC with kserve enabled
• check for errors in opendatahub-operator-controller-manager and odh-model-controller log

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

[VedantMahabaleshwarkar]

@ykaliuta we would still have a race condition due to modelmesh even with this PR, right?
e.g:

• Modelmesh and Kserve and both Managed
• Modelmesh reconciliation is still independent of servicemesh features, so the odh-model-controller rollout might still complete before the kserve+dependency setup is complete.
• Here odh-model-controller would still run into the same error.

@israel-hdez can correct me if I'm wrong, but in addition to these changes I believe we also wanted to restart the odh-model-controller pods when the management state for kserve is modified. This way even if odh-model-controller exists in an incorrect state (due to modelmesh), it will be corrected when the pods restart

[ykaliuta]

@ykaliuta we would still have a race condition due to modelmesh even with this PR, right? e.g:

• Modelmesh and Kserve and both Managed
• Modelmesh reconciliation is still independent of servicemesh features, so the odh-model-controller rollout might still complete before the kserve+dependency setup is complete.
• Here odh-model-controller would still run into the same error.

Correct, but odh-model-controller implements another check, I mentioned it in the description.

@israel-hdez can correct me if I'm wrong, but in addition to these changes I believe we also wanted to restart the odh-model-controller pods when the management state for kserve is modified. This way even if odh-model-controller exists in an incorrect state (due to modelmesh), it will be corrected when the pods restart

You are right, but changing kserve management state is out of scope of the issue. If restart is supposed to happen when the state changes then it is not the case here, the state stays the same.

[israel-hdez]

@ykaliuta we would still have a race condition due to modelmesh even with this PR, right? e.g:

• Modelmesh and Kserve and both Managed
• Modelmesh reconciliation is still independent of servicemesh features, so the odh-model-controller rollout might still complete before the kserve+dependency setup is complete.
• Here odh-model-controller would still run into the same error.

Correct, but odh-model-controller implements another check, I mentioned it in the description.

I think part of the problem is that both the DSCInitialization and the DataScienceCluster resources can reconcile in parallel.

Because of ModelMesh, the odh-model-controller would still boot, while the reconciliation of the DSCInitialization is still installing OSSM and Authorino. Thus, there is still the chance of a race condition if the Authorino CRD hasn't been created yet.

So, despite the changes to odh-model-controller and this PR would be fixing a race condition over the AuthorizationPolicy, we still have another race condition over Authorino CRDs creation.

@israel-hdez can correct me if I'm wrong, but in addition to these changes I believe we also wanted to restart the odh-model-controller pods when the management state for kserve is modified. This way even if odh-model-controller exists in an incorrect state (due to modelmesh), it will be corrected when the pods restart

You are right, but changing kserve management state is out of scope of the issue. If restart is supposed to happen when the state changes then it is not the case here, the state stays the same.

I'm not sure if it is really out of scope. Users would be unable to query models in such situation and the title of the Jira would still describe the issue.

[ykaliuta]

@ykaliuta we would still have a race condition due to modelmesh even with this PR, right? e.g:

• Modelmesh and Kserve and both Managed
• Modelmesh reconciliation is still independent of servicemesh features, so the odh-model-controller rollout might still complete before the kserve+dependency setup is complete.
• Here odh-model-controller would still run into the same error.

Correct, but odh-model-controller implements another check, I mentioned it in the description.

I think part of the problem is that both the DSCInitialization and the DataScienceCluster resources can reconcile in parallel.

Because of ModelMesh, the odh-model-controller would still boot, while the reconciliation of the DSCInitialization is still installing OSSM and Authorino. Thus, there is still the chance of a race condition if the Authorino CRD hasn't been created yet.

I do not see problem here, correct me if I'm wrong. AuthConfigs CRD (not CR) is created when Authorino operator is installed (supposed to be done manually before ODH/RHOAI as pre-requisite). Regardless of the CRs odh-model-controller will work (will able to watch it), it will just do nothing until operator deploys configuration, if any.

The CRD stays there if Authorino is uninstalled later. But operators checks for subscription before deploying the configuration.

So, despite the changes to odh-model-controller and this PR would be fixing a race condition over the AuthorizationPolicy, we still have another race condition over Authorino CRDs creation.

@israel-hdez can correct me if I'm wrong, but in addition to these changes I believe we also wanted to restart the odh-model-controller pods when the management state for kserve is modified. This way even if odh-model-controller exists in an incorrect state (due to modelmesh), it will be corrected when the pods restart

You are right, but changing kserve management state is out of scope of the issue. If restart is supposed to happen when the state changes then it is not the case here, the state stays the same.

I'm not sure if it is really out of scope. Users would be unable to query models in such situation and the title of the Jira would still describe the issue.

May be Jira should be renamed. Because inside it describes just the race during fresh install, not changing the state afterwards.

[israel-hdez]

I do not see problem here, correct me if I'm wrong. AuthConfigs CRD (not CR) is created when Authorino operator is installed (supposed to be done manually before ODH/RHOAI as pre-requisite).

You are right. I stand corrected.

I'm not sure if it is really out of scope. Users would be unable to query models in such situation and the title of the Jira would still describe the issue.

May be Jira should be renamed. Because inside it describes just the race during fresh install, not changing the state afterwards.

In the comments, it was identified as something that needs to be fixed.
I'll let @Jooho @VaishnaviHire and @mwaykole to comment if such situation should also be fixed here, or if another ticket needs to be created.

[israel-hdez]

cc @mattmahoneyrh , because he is QA contact ^read previous^

[bartoszmajsak]

It's better to first set up infra bits required for this component before deploying it.

I left one small suggestion around error handling, but other than that LGTM.

[bartoszmajsak]

LGTM

[ykaliuta]

/retest-required

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bartoszmajsak, zdtsw

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

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