✨ Add optional field to ClusterExtension API to skip CRD upgrade safety checks by rashmigottipati · Pull Request #845 · operator-framework/operator-controller

rashmigottipati · 2024-05-10T16:33:15Z

Description

Adds an optional ClusterExtension.spec.skipCRDUpgradeSafetyChecks field that when set to true will be used to disable the CRD upgrade safety checks when attempting to install a cluster extension.

No logic included in this PR as the CRD Upgrade Safety preflight check as there isn't anything yet to enable/disable. So logic to enable/disable will be added in a future PR once everything is in place.

fixes#745

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2024-05-10T16:33:31Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`fffc98a`
🔍 Latest deploy log	https://app.netlify.com/sites/olmv1/deploys/6643b1c826b7530008dd9348
😎 Deploy Preview	https://deploy-preview-845--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2024-05-10T17:04:08Z

Codecov Report

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

Project coverage is 70.05%. Comparing base (e9acbd3) to head (e35ad16).

Files	Patch %	Lines
api/v1alpha1/zz_generated.deepcopy.go	0.00%	18 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #845 +/- ##
==========================================
- Coverage 71.02% 70.05% -0.97% 
==========================================
Files 17 17 Lines 1301 1319 +18 ==========================================
Hits 924 924 - Misses 304 322 +18 
Partials 73 73

Flag	Coverage Δ
e2e	`43.36% <0.00%> (-0.60%)`	⬇️
unit	`62.66% <0.00%> (-0.93%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

… checks Signed-off-by: Rashmi Gottipati <chowdary.grashmi@gmail.com>

Signed-off-by: Rashmi Gottipati <chowdary.grashmi@gmail.com>

everettraven · 2024-05-14T20:17:05Z

+	//+kubebuilder:validation:Enum:=Enabled;Disabled
+	//+kubebuilder:default:=Enabled
+	//+kubebuilder:Optional
+	CRDUpgradeSafetyMode CRDUpgradeSafetyMode `json:"mode,omitempty"`


nit:
Suggested change
CRDUpgradeSafetyModeCRDUpgradeSafetyMode`json:"mode,omitempty"`
ModeCRDUpgradeSafetyMode`json:"mode,omitempty"`

everettraven

One minor nit, but not worth blocking this PR from merging IMO. Thanks @rashmigottipati !

joelanford · 2024-05-14T20:41:23Z

+
+	//+kubebuilder:Optional
+	// Preflight defines the strategy for the preflight checks (i.e. as of now, the CRD upgrade safety checks) to be applied or skipped when attempting to install the cluster extension.
+	Preflight PreflightConfig `json:"preflight,omitempty"`


If a field is optional (i.e. with omitempty), it should also be a pointer.

Ah good catch - I totally missed that!

joelanford · 2024-05-14T20:43:09Z

+// PreflightConfig holds the config for the preflight checks. Currently, this is implemented for the
+// CRDUpgradeSafety preflight check and can be extended in the future to add other preflight checks.
+type PreflightConfig struct {
+	CRDUpgradeSafety CRDUpgradeSafetyPreflightConfig `json:"crdUpgradeSafety,omitempty"`


Same comment as above: re using pointer if field is optional.

varshaprasad96 · 2024-05-15T15:52:05Z

Please hold this from merging till #846 gets in. Thanks!

rashmigottipati · 2024-05-15T18:08:33Z

/hold

openshift-merge-robot · 2024-06-14T20:25:35Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

joelanford · 2024-07-03T20:30:22Z

@rashmigottipati should we close this? Am I correct in assuming that #979 makes this obsolete?

rashmigottipati · 2024-07-03T20:38:24Z

@joelanford yes, #979 makes this obsolete. Will close this

rashmigottipati requested a review from everettraven May 10, 2024 16:33

rashmigottipati self-assigned this May 10, 2024

rashmigottipati requested a review from a team as a code owner May 10, 2024 16:33

rashmigottipati force-pushed the configure-crd-checks branch from 63d53f0 to 580c351Compare May 10, 2024 16:53

everettraven reviewed May 10, 2024
View reviewed changes