Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: pass platform from env variable or fall back to use old logic #1252

Merged

Conversation

zdtsw
Copy link
Member

@zdtsw zdtsw commented Sep 19, 2024

Description

feat: pass platform from env variables or fall back to use old logic

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed

introduce new env var OPERATOR_RELEASE_VERSION, value also set during build time
if value is empty, fall back to use old way from CSV to read version or use 0.0.0
add support from makefile
use envstubst to replace version
(split from #1231)

How Has This Been Tested?

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

@zdtsw zdtsw requested review from lburgazzoli and ykaliuta and removed request for Sara4994 and jackdelahunt September 19, 2024 13:00
@ykaliuta
Copy link
Contributor

I support the change. And it is backward compatible. Just worried about the formal process now.

@zdtsw
Copy link
Member Author

zdtsw commented Sep 19, 2024

i can markt it to [draft] for now and wait to see how / when we want it

@ykaliuta
Copy link
Contributor

ykaliuta commented Oct 7, 2024

I support the change. And it is backward compatible. Just worried about the formal process now.

It was about original version where only platform was passed with environment.
Version is a different piece of information and I do not support using environment for that (it was already discussed in some other PR)

@zdtsw
Copy link
Member Author

zdtsw commented Oct 8, 2024

I support the change. And it is backward compatible. Just worried about the formal process now.

It was about original version where only platform was passed with environment. Version is a different piece of information and I do not support using environment for that (it was already discussed in some other PR)

i do not tend to get the version part out, but to run the full tests, i will need to get it from draft to PR to trigger ci.
for the "it was already discussed in some other PR", maybe i missed/dont recall that part , do you have a reference where the converstation ended?

@ykaliuta
Copy link
Contributor

ykaliuta commented Oct 8, 2024

i do not tend to get the version part out, but to run the full tests, i will need to get it from draft to PR to trigger ci. for the "it was already discussed in some other PR", maybe i missed/dont recall that part , do you have a reference where the converstation ended?

Jira (13115) and probably slack actually. Looks like was ignored.

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed
- introduce new env var OPERATOR_RELEASE_VERSION, value also set during build time
  - if value is empty, fall back to use old way from CSV to read version or use 0.0.0
- add support from makefile
  - use envstubst to replace version

Signed-off-by: Wen Zhou <[email protected]>
@zdtsw
Copy link
Member Author

zdtsw commented Oct 9, 2024

i do not tend to get the version part out, but to run the full tests, i will need to get it from draft to PR to trigger ci. for the "it was already discussed in some other PR", maybe i missed/dont recall that part , do you have a reference where the converstation ended?

Jira (13115) and probably slack actually. Looks like was ignored.

i removed the part for the release version

Copy link

openshift-ci bot commented Oct 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ykaliuta

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:

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

@openshift-ci openshift-ci bot added the approved label Oct 9, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 283d350 into opendatahub-io:incubation Oct 9, 2024
8 checks passed
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.

Please upload report for BASE (incubation@6134496). Learn more about missing BASE report.

Files with missing lines Patch % Lines
pkg/cluster/cluster_config.go 0.00% 12 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             incubation    #1252   +/-   ##
=============================================
  Coverage              ?   19.09%           
=============================================
  Files                 ?       30           
  Lines                 ?     2645           
  Branches              ?        0           
=============================================
  Hits                  ?      505           
  Misses                ?     2079           
  Partials              ?       61           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

VaishnaviHire added a commit that referenced this pull request Oct 9, 2024
* cluster_config: init cluster variables on startup (#1059)

* cluster_config: move type definitions to the beginning of the file

Just a bit more clarity

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

* cluster_config: init cluster variables on startup

Cluster configuration is supposed to be the same during operator pod
lifetime. There is no point to detect it on every reconcile cycle
keeping in mind that it can causes many api requests.

Do the initialization on startup. Keep GetOperatorNamespace()
returning error since it defines some logic in the DSCInitialization
reconciler.

Automatically called init() won't work here due to need to check
error of the initialization.

Wrap logger into context and use it in the Init() like
controller-runtime does [1][2].

Save root context without the logger for mgr.Start since "setup"
logger does not fit normal manager's work.

Leave GetDomain() as is since it uses OpenshiftIngress configuration
which is created when DSCInitialization instantiated.

Log cluster configuration on success from Init, so remove platform
logging from main.

[1] https://github.com/kubernetes-sigs/controller-runtime/blob/38546806f2faf5973e3321a7bd5bb3afdbb5767d/pkg/internal/controller/controller.go#L297
[2] https://github.com/kubernetes-sigs/controller-runtime/blob/38546806f2faf5973e3321a7bd5bb3afdbb5767d/pkg/internal/controller/controller.go#L111

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

* cluster: do not return error from GetRelease

GetRelease return values are defined at startup, the error checked
in main, so no point to return error anymore.

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

---------

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

* components: move params.env image updating to Init stage (#1191)

* components, main: add Component Init method

Add Init() method to the Component interface and call it from main on
startup.

Will be used to move startup-time code from ReconcileComponent
(like adjusting params.env).

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

* components: move params.env image updating to Init stage

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

Image names in environment are not supposed to be changed during
runtime of the operator, so it makes sense to update them only on
startup.

If manifests are overriden by DevFlags, the DevFlags' version will
be used.

The change is straight forward for most of the components where only
images are updated and params.env is located in the kustomize root
directory, but some components (dashboard, ray, codeflare,
modelregistry) also update some extra parameters. For them image
part only is moved to Init since other updates require runtime DSCI
information.

The patch also changes logic for ray, codeflare, and modelregistry
in this regard to update non-image parameters regardless of DevFlags
like it was changed in dashboard recently.

The DevFlags functionality brings some concerns:

- For most components the code is written such a way that as soon as
DevFlags supplied the global path variables are changed and never
reverted back to the defaults. For some (dashboard, trustyai) there
is (still global) OverridePath/entryPath pair and manifests reverted
to the default, BUT there is no logic of transition.

- codeflare: when manifests are overridden namespace param is
updated in the hardcoded (stock) path;

This logic is preserved.

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

---------

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

* update: remove webhook service from bundle (#1275)

- we do not need it in bundle, CSV auto generate one during installation
- if we install operator via OLM, webhook service still get used from config/webhook/service.yaml

Signed-off-by: Wen Zhou <[email protected]>

* update: add validation on application and monitoring namespace in DSCI (#1263)

Signed-off-by: Wen Zhou <[email protected]>

* logger: add zap command line switches (#1280)

Allow to tune preconfigured by --log-mode zap backend with standard
zap command line switches from controller-runtime (zap-devel,
zap-encoder, zap-log-level, zap-stacktrace-level,
zap-time-encoding)[1].

This brings flexibility in logger setup for development environments
first of all.

The patch does not change default logger setup and does not change
DSCI override functionality.

[1] https://sdk.operatorframework.io/docs/building-operators/golang/references/logging

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

* Modify Unit Tests GitHub Actions workflow to get code coverage test reports (#1279)

* Create codecov.yml

* Added to run test coverage also on PRs

* Removing trailing ]

* Update .github/workflows/codecov.yml

Co-authored-by: Wen Zhou <[email protected]>

* Removed go mod install dependency

* Consolidated codecov workflow into unit tests workflow

---------

Co-authored-by: Wen Zhou <[email protected]>

* webhook: move initialization inside of the module (#1284)

Add webhook.Init() function and hide webhook setup inside of the
module. It will make it possible to replace Init with a NOP (no
operation) with conditional compilation for no-webhook build.

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

* feat: pass platform from env variable or fall back to use old logic (#1252)

* feat: pass platform from env variables or fall back to use old logic

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed
- introduce new env var OPERATOR_RELEASE_VERSION, value also set during build time
  - if value is empty, fall back to use old way from CSV to read version or use 0.0.0
- add support from makefile
  - use envstubst to replace version

Signed-off-by: Wen Zhou <[email protected]>

* update: remove release version in the change

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>

* fix: update release version in DSCI and DSC .status for upgrade case (#1287)

- DSCI: if current version is not matching, update it
- DSC: in both reconcile pass and fail case, update it

Signed-off-by: Wen Zhou <[email protected]>

* Update version to 2.19.0 (#1291)

Co-authored-by: VaishnaviHire <[email protected]>

---------

Signed-off-by: Yauheni Kaliuta <[email protected]>
Signed-off-by: Wen Zhou <[email protected]>
Co-authored-by: Yauheni Kaliuta <[email protected]>
Co-authored-by: Wen Zhou <[email protected]>
Co-authored-by: Adrián Sanz Gómiz <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: VaishnaviHire <[email protected]>
zdtsw added a commit to zdtsw-forking/rhods-operator that referenced this pull request Oct 10, 2024
…pendatahub-io#1252)

* feat: pass platform from env variables or fall back to use old logic

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit 283d350)
zdtsw added a commit to zdtsw-forking/rhods-operator that referenced this pull request Oct 10, 2024
…pendatahub-io#1252)

* feat: pass platform from env variables or fall back to use old logic

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit 283d350)
zdtsw added a commit to zdtsw-forking/rhods-operator that referenced this pull request Oct 17, 2024
…pendatahub-io#1252)

* feat: pass platform from env variables or fall back to use old logic

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit 283d350)
zdtsw added a commit to zdtsw-forking/rhods-operator that referenced this pull request Oct 17, 2024
…pendatahub-io#1252)

* feat: pass platform from env variables or fall back to use old logic

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit 283d350)
openshift-merge-bot bot pushed a commit to red-hat-data-services/rhods-operator that referenced this pull request Oct 17, 2024
…pendatahub-io#1252)

* feat: pass platform from env variables or fall back to use old logic

- introduce new env var ODH_PLATFORM_TYPE, value set during build time
  - if value not match, fall back to detect managed then self-managed

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit 283d350)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants