[rhoai-2.9] Sync upstream 2.9 #240
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: npecka <[email protected]>
This reverts commit 41a643f.
Revert "feat(DW): prepare CF,ray,kueue for GA (opendatahub-io#929)"
Altering alert from 0.99950 to 0.98
(cherry picked from commit d3b7a59)
…kcer (opendatahub-io#952) * feat(cleanup): remove -auth-provicer namespace when remove FTer - add OwnedBy() into namespace creation step as precondition - add check in testcase --------- Signed-off-by: Wen Zhou <[email protected]> Co-authored-by: Bartosz Majsak <[email protected]> (cherry picked from commit 562ae3d)
Signed-off-by: Wen Zhou <[email protected]> (cherry picked from commit a07296e)
…hub-io#949) * feat(authz): conditionally enable authorization capability Instead of failing when Authorino operator is not present, features releated to this capability won't be installed. This allows to handle previously existings RHOAI installation where KServe does not have yet authorization module bundled. Once Authorino operator is installed, there needs to be reconcile triggered so that authz-capability features can be configured. How to test - use custom images for odh-operator (and odh-model-controller) - quay.io/bmajsak/opendatahub-operator:dev-authz-soft-opt-in - quay.io/bmajsak/odh-model-controller:latest (reference updated in the odh operator image) - note: odh-operator image manifests point to `main` branch of odh-model-controller - install all prerequisites BUT Authorino Operator - notice there's no `auth-provider` ns nor authorino-related configuration for service mesh - enable KServe in DSC - test sample isvc (e.g. by using [this script](https://gist.githubusercontent.com/israel-hdez/af374562ef9e5b9d80890aa6f0bce20d/raw/5651e4d7c35be51751049c3c2a236771e9005ace/kserve-sample-model-test.sh)) - observe 200 - install Authorino operator - trigger reconcile of DSCI and DSC so changes are applied both to cluster setup and component - restart `odh-model-controller` pod - trigger sample script again - observe 200 - check Authorino logs and see anonymous access was granted - enable authorization on isvc - i.e. `oc annotate isvc sklearn-v2-iris security.opendatahub.io/enable-auth=true` - call isvc again - observe 401 - add token to isvc call's Header - observe 200 Requires opendatahub-io/odh-model-controller#183 to work end-to-end. * feat: introduces generic status reporter struct Introducing generic struct Reporter to handle status/condition updates. It evolved from `updateStatus` funcs available in DSCI and DSC controllers (and these are also moved to a single generic func now). New reporter struct has single responsibility - it allows to define how condition should be updated. Developer defines it in `CalculateCondition` closure where optional error will be passed for inspection. This closure should return known from previous incarnation `update func(saved)` which will append target object's conditions. The only difference is access to optional error. The main goal for introducing this approach is to avoid catch-error-on-defer, where we rely on named return parameter and mutate condition directly in the function which performs actions based on the resource desired state. Example: ```golang func createReporter(condition *conditionsv1.Condition) *status.Reporter[*dsciv1.DSCInitialization] { return status.NewStatusReporter[*dsciv1.DSCInitialization]( func(err error) status.SaveStatusFunc[*dsciv1.DSCInitialization] { return func(saved *dsciv1.DSCInitialization) { if err != nil { condition.Status = corev1.ConditionFalse condition.Message = err.Error() condition.Reason = status.CapabilityFailed var missingOperatorErr *feature.MissingOperatorError if errors.As(err, &missingOperatorErr) { condition.Reason = status.MissingOperatorReason } } conditionsv1.SetStatusCondition(&saved.Status.Conditions, *condition) } }, ) } ``` Then to call it: ```golang func (r *DSCInitializationReconciler) doServiceMeshStuff(instance *dsciv1.DSCInitialization) error { reporter := createReporter(&conditionsv1.Condition{ Type: status.CapabilityServiceMesh, Status: corev1.ConditionFalse, Reason: status.ConfiguredReason, Message: "Service Mesh configured", }) serviceMeshErr := createServiceMesh(instance) _, reportError := reporter.ReportCondition(r.Client, instance, serviceMeshErr) return reportError } ``` Next step: migrate FeatureTracker to this new API Signed-off-by: bartoszmajsak <[email protected]> * feat(authz): reports missing authorino operator without failure When Authorino Operator is missing the reconcile will continue without throwing an error about missing pre-requisite and asking user to install it. This fact is instead reported as DSCI.status condition: ```yaml - lastHeartbeatTime: "2024-04-04T16:32:22Z" lastTransitionTime: "2024-04-04T16:24:38Z" message: Authorino operator is not installed on the cluster, skipping authorization capability reason: MissingOperator status: "False" type: CapabilityServiceMeshAuthorization ``` This allows existing KServe installations which do not have authorization capability to migrate smoothly and continue working without authz layer - so to allow for soft opt-in for Authz layer leveraging Authorino, as there is no concensus on automated installation of this dependent operator on behalf of the user. This commit introduces a notion of capability, though not fully fleshed out as a concept - only condition type for now. It is necessery to split set of service mesh features to core and authz so that they not only can be reported separately but can be handled on their own. This is needed for so called "soft opt-in" mentioned above. Signed-off-by: bartoszmajsak <[email protected]> * fix(error): uses single %w wrap instead of 2 It seems that go1.19 does not support wrapping two errors in the same fmt.Errorf call, whereas go1.20 does :) Simplifying the code to have single error tree and make go1.19 compiler happy. Signed-off-by: bartoszmajsak <[email protected]> * chore: makes Reporter self-contained - adds client and instance to which status is attached as part of the struct - changes how HandleWithReporter is used - now it's a wrapper around Apply/Delete which calls reporter underneath instead Signed-off-by: bartoszmajsak <[email protected]> * chore: uses `Println` to WARN Co-authored-by: Wen Zhou <[email protected]> * chore: minor godoc and err improvements Signed-off-by: bartoszmajsak <[email protected]> --------- Signed-off-by: bartoszmajsak <[email protected]> Co-authored-by: Wen Zhou <[email protected]> (cherry picked from commit 93e2126)
VaishnaviHire
merged commit b9bbecd
into
red-hat-data-services:rhoai-2.9
5 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sync upstream changes in rhoai-2.9
Changes include
#235
#234
opendatahub-io#954
opendatahub-io#949