webhook: move initialization inside of the module #1284
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
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 <ykaliuta@redhat.com>
|
/cc @zdtsw @lburgazzoli |
This was referenced Oct 9, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## incubation #1284 +/- ##
=============================================
Coverage ? 19.10%
=============================================
Files ? 30
Lines ? 2643
Branches ? 0
=============================================
Hits ? 505
Misses ? 2077
Partials ? 61 ☔ View full report in Codecov by Sentry. |
lburgazzoli
approved these changes
Oct 9, 2024
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lburgazzoli 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 |
openshift-merge-bot
bot
merged commit e40c770
into
opendatahub-io:incubation
8 checks passed
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 <ykaliuta@redhat.com> * 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 <ykaliuta@redhat.com> * 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 <ykaliuta@redhat.com> --------- Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> * 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 <ykaliuta@redhat.com> * 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 <ykaliuta@redhat.com> --------- Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> * 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 <wenzhou@redhat.com> * update: add validation on application and monitoring namespace in DSCI (#1263) Signed-off-by: Wen Zhou <wenzhou@redhat.com> * 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 <ykaliuta@redhat.com> * 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 <wenzhou@redhat.com> * Removed go mod install dependency * Consolidated codecov workflow into unit tests workflow --------- Co-authored-by: Wen Zhou <wenzhou@redhat.com> * 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 <ykaliuta@redhat.com> * 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 <wenzhou@redhat.com> * update: remove release version in the change Signed-off-by: Wen Zhou <wenzhou@redhat.com> --------- Signed-off-by: Wen Zhou <wenzhou@redhat.com> * 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 <wenzhou@redhat.com> * Update version to 2.19.0 (#1291) Co-authored-by: VaishnaviHire <17230536+VaishnaviHire@users.noreply.github.com> --------- Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> Signed-off-by: Wen Zhou <wenzhou@redhat.com> Co-authored-by: Yauheni Kaliuta <ykaliuta@redhat.com> Co-authored-by: Wen Zhou <wenzhou@redhat.com> Co-authored-by: Adrián Sanz Gómiz <100594859+asanzgom@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: VaishnaviHire <17230536+VaishnaviHire@users.noreply.github.com>
zdtsw
pushed a commit
to zdtsw-forking/rhods-operator
that referenced
this pull request
Oct 18, 2024
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 <ykaliuta@redhat.com> (cherry picked from commit e40c770)
5 tasks
openshift-merge-bot bot
pushed a commit
to red-hat-data-services/rhods-operator
that referenced
this pull request
Oct 18, 2024
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 <ykaliuta@redhat.com> (cherry picked from commit e40c770)
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.
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.
Description
How Has This Been Tested?
Screenshot or short clip
Merge criteria