[RFC] feat: pass platform from env variable and fall back to use old logic

[zdtsw]

Description

• introduce new env var ODH_PLATFORM_TYPE
• rename old function to DetectPlatform
• create new GetPlatform to check env first or fall to call DetectPlatform
• move old call cluster.GetPlatform, either pass as parameter or out of subcomponent reconcile

How Has This Been Tested?

local build: quay.io/wenzhou/opendatahub-operator-catalog:v2.18.913-0 without setting new env == the old way
operator pod: print {"level":"info","ts":"2024-09-13T11:07:12Z","logger":"setup","msg":"running on","platform":"Open Data Hub"}
by adding in CSV

- name: PLATFORM
  value: SelfManagedRHOAI

oprator pod print {"level":"info","ts":"2024-09-13T11:06:38Z","logger":"setup","msg":"running on","platform":"OpenShift AI Self-Managed"}

Screenshot or short clip

Merge criteria

• You have read the contributors guide.
• Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
• Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
• 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 NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from zdtsw. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

[ykaliuta]

The patch contains much more changes than in description

[lburgazzoli]

quck note: ideally, env vars should have a domain prefix to avoid any conflict, i.e. ODH_

[zdtsw]

quck note: ideally, env vars should have a domain prefix to avoid any conflict, i.e. ODH_

but we are having 2 different env variable in upstream and downstream: ODH_PLATFORM and RHOAI_PLATFORM
or, we have ODH_PLATFORM for both?

[zdtsw]

The patch contains much more changes than in description

the main change is the "env part" but since it reuses the old function name, then the old function have to get a new name.

as for the part of "removing duplicated calls (of old function) and use parameters" it is not stated in the draft's title, only in the description.

[ykaliuta]

The patch contains much more changes than in description

the main change is the "env part" but since it reuses the old function name, then the old function have to get a new name.

as for the part of "removing duplicated calls (of old function) and use parameters" it is not stated in the draft's title, only in the description.

I see the change as you keep API the same and only change implementation under the hood.
I have #1059 then I cannot rebase to discuss.

[lburgazzoli]

quck note: ideally, env vars should have a domain prefix to avoid any conflict, i.e. ODH_

but we are having 2 different env variable in upstream and downstream: ODH_PLATFORM and RHOAI_PLATFORM or, we have ODH_PLATFORM for both?

I'd say ODH_ shoud be enough, my understanding is that i.e. for related images we still use ODH, like https://github.com/red-hat-data-services/rhods-operator/blob/76acc49b4e14b5ad6e63c8bdd8c194c9335c417c/components/ray/ray.go#L62C45-L62C63

[ykaliuta]

In general, I support passing data to functions explicitly instead of look up from some global state inside. As much as possible. Usually it in addition makes functions more testable. But it is just a different change.

[ykaliuta]

Can it be splitted into 2 PRs -- passing platform as a parameter, and introducing environment variable?
If the first one is just a simple refactoring, the second changes functionality and deserves more attention/discussion.

[zdtsw]

Can it be splitted into 2 PRs -- passing platform as a parameter, and introducing environment variable? If the first one is just a simple refactoring, the second changes functionality and deserves more attention/discussion.

can do that.
can get the refactor out first, and leave the "env" part once we are ready for "refinement"

[zdtsw]

close in favor of 2 following PRs