From adc096b6a31af24f17bb9178517e56a8afed462d Mon Sep 17 00:00:00 2001 From: Juan Osorio Robles Date: Fri, 28 Aug 2020 22:50:17 +0300 Subject: [PATCH 1/9] Make bundle validate subcommand respect verbosity (#3795) * Make bundle validate subcommand respect verbosity This makes the bundle validate subcommand respect the verbosity level by setting it directly in the logger that's actually used, and not in the global logger as was done previously. Closes: #3793 * add unit test Co-authored-by: Austin Macdonald --- changelog/fragments/bundle-validate-verbose.yaml | 4 ++++ internal/cmd/operator-sdk/bundle/validate.go | 16 ++++++++++------ .../cmd/operator-sdk/bundle/validate_test.go | 14 ++++++++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 changelog/fragments/bundle-validate-verbose.yaml diff --git a/changelog/fragments/bundle-validate-verbose.yaml b/changelog/fragments/bundle-validate-verbose.yaml new file mode 100644 index 00000000000..0a74e61e154 --- /dev/null +++ b/changelog/fragments/bundle-validate-verbose.yaml @@ -0,0 +1,4 @@ +entries: + - description: Fixed debug logging in the `bundle validate` subcommand of `operator-sdk` + kind: "bugfix" + breaking: false diff --git a/internal/cmd/operator-sdk/bundle/validate.go b/internal/cmd/operator-sdk/bundle/validate.go index 334b467f4a9..054db313ea5 100644 --- a/internal/cmd/operator-sdk/bundle/validate.go +++ b/internal/cmd/operator-sdk/bundle/validate.go @@ -93,16 +93,11 @@ func makeValidateCmd() *cobra.Command { Use: "validate", Short: "Validate an operator bundle", RunE: func(cmd *cobra.Command, args []string) (err error) { - if viper.GetBool(flags.VerboseOpt) { - log.SetLevel(log.DebugLevel) - } - // Always print non-output logs to stderr as to not pollute actual command output. // Note that it allows the JSON result be redirected to the Stdout. E.g // if we run the command with `| jq . > result.json` the command will print just the logs // and the file will have only the JSON result. - logger := log.NewEntry(internal.NewLoggerTo(os.Stderr)) - + logger := createLogger(viper.GetBool(flags.VerboseOpt)) if err = c.validate(args); err != nil { return fmt.Errorf("invalid command args: %v", err) } @@ -126,6 +121,15 @@ func makeValidateCmd() *cobra.Command { return cmd } +// createLogger creates a new logrus Entry that is optionally verbose. +func createLogger(verbose bool) *log.Entry { + logger := log.NewEntry(internal.NewLoggerTo(os.Stderr)) + if verbose { + logger.Logger.SetLevel(log.DebugLevel) + } + return logger +} + // validate verifies the command args func (c bundleValidateCmd) validate(args []string) error { if len(args) != 1 { diff --git a/internal/cmd/operator-sdk/bundle/validate_test.go b/internal/cmd/operator-sdk/bundle/validate_test.go index 8eeca44bb97..2ccded45c52 100644 --- a/internal/cmd/operator-sdk/bundle/validate_test.go +++ b/internal/cmd/operator-sdk/bundle/validate_test.go @@ -18,6 +18,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/operator-framework/operator-sdk/internal/cmd/operator-sdk/bundle/internal" + log "github.com/sirupsen/logrus" ) var _ = Describe("Running a bundle validate command", func() { @@ -38,6 +39,19 @@ var _ = Describe("Running a bundle validate command", func() { }) }) + Describe("Creating a logger", func() { + It("that is Info Level when not verbose", func() { + verbose := false + logger := createLogger(verbose) + Expect(logger.Logger.GetLevel()).To(Equal(log.InfoLevel)) + }) + It("that is Debug level if verbose", func() { + verbose := true + logger := createLogger(verbose) + Expect(logger.Logger.GetLevel()).To(Equal(log.DebugLevel)) + }) + }) + Describe("validate", func() { var cmd bundleValidateCmd BeforeEach(func() { From 43c3ea88f656139d729357472aa2e6cde2ccd848 Mon Sep 17 00:00:00 2001 From: Venkat Ramaraju <37827279+VenkatRamaraju@users.noreply.github.com> Date: Tue, 11 Aug 2020 13:54:04 -0700 Subject: [PATCH 2/9] Adding checks for rescued tasks during reconciliation (#3650) * Stopping reconciliation on rescue * Added error check --- internal/ansible/controller/reconcile.go | 2 +- internal/ansible/runner/eventapi/types.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/ansible/controller/reconcile.go b/internal/ansible/controller/reconcile.go index ee2d7fdff42..d6945561313 100644 --- a/internal/ansible/controller/reconcile.go +++ b/internal/ansible/controller/reconcile.go @@ -186,7 +186,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(request reconcile.Request) (reconc return reconcile.Result{}, err } } - if event.Event == eventapi.EventRunnerOnFailed && !event.IgnoreError() { + if event.Event == eventapi.EventRunnerOnFailed && !event.IgnoreError() && !event.Rescued() { failureMessages = append(failureMessages, event.GetFailedPlaybookMessage()) } } diff --git a/internal/ansible/runner/eventapi/types.go b/internal/ansible/runner/eventapi/types.go index f8098da3e88..d0ba259e7b2 100644 --- a/internal/ansible/runner/eventapi/types.go +++ b/internal/ansible/runner/eventapi/types.go @@ -122,3 +122,15 @@ func (je JobEvent) IgnoreError() bool { } return false } + +// Rescued - Detects whether or not a task was rescued +func (je JobEvent) Rescued() bool { + if rescued, contains := je.EventData["rescued"]; contains { + for _, v := range rescued.(map[string]interface{}) { + if int(v.(float64)) == 1 { + return true + } + } + } + return false +} From e7f3dfb101064ebd1d7b6712ca0f34320a339d06 Mon Sep 17 00:00:00 2001 From: Venkat Ramaraju <37827279+VenkatRamaraju@users.noreply.github.com> Date: Thu, 13 Aug 2020 07:33:23 -0700 Subject: [PATCH 3/9] Adding fragment file for PR #3650 (Rescue tasks reconciliation) (#3729) * Added fragments file * Fragment file * Changes --- changelog/fragments/rescue-reconciliation.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/fragments/rescue-reconciliation.yaml diff --git a/changelog/fragments/rescue-reconciliation.yaml b/changelog/fragments/rescue-reconciliation.yaml new file mode 100644 index 00000000000..0424f7a867e --- /dev/null +++ b/changelog/fragments/rescue-reconciliation.yaml @@ -0,0 +1,9 @@ +entries: + - description: > + Stop reconciling tasks when the event raised is a rescue in Ansible-based Operators. + More info: [Bugzilla 1856714](https://bugzilla.redhat.com/show_bug.cgi?id=1856714) + + kind: "bugfix" + + breaking: false + pull_request_override: 3650 From 8f53c74892195c9416a4b49d2383f65a34a7bd0b Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 18 Aug 2020 14:50:37 -0400 Subject: [PATCH 4/9] `generate`: support only AllNamespaces by default in CSV bases (#3746) internal/generate/clusterserviceversion/bases: set only `AllNamespaces` to `supported: true` by default *: update examples and testdata docs/building-operators/golang/operator-scope.md: add section discussing namespaced CSV --- .../fragments/csv-allnamespaces-default.yaml | 5 + .../bases/clusterserviceversion.go | 4 +- ...cached-operator.clusterserviceversion.yaml | 4 +- ...ith-ui-metadata.clusterserviceversion.yaml | 4 +- ...cached-operator.clusterserviceversion.yaml | 4 +- ...cached-operator.clusterserviceversion.yaml | 8 +- internal/scorecard/testdata/bundle.tar.gz | Bin 2762 -> 2746 bytes ...cached-operator.clusterserviceversion.yaml | 8 +- .../golang/operator-scope.md | 116 ++++++++++++------ 9 files changed, 99 insertions(+), 54 deletions(-) create mode 100644 changelog/fragments/csv-allnamespaces-default.yaml diff --git a/changelog/fragments/csv-allnamespaces-default.yaml b/changelog/fragments/csv-allnamespaces-default.yaml new file mode 100644 index 00000000000..ecaa45a4d0e --- /dev/null +++ b/changelog/fragments/csv-allnamespaces-default.yaml @@ -0,0 +1,5 @@ +entries: + - description: > + `generate ` now generates a CSV base with only the `AllNamespaces` install mode + supported by default, since projects are cluster-scoped by default. + kind: bugfix diff --git a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go index 7b420ce3d73..e7c7a92b7a6 100644 --- a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go +++ b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go @@ -161,8 +161,8 @@ func (b ClusterServiceVersion) makeNewBase() *v1alpha1.ClusterServiceVersion { Keywords: b.Keywords, Icon: b.Icon, InstallModes: []v1alpha1.InstallMode{ - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: false}, {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, }, diff --git a/internal/generate/testdata/clusterserviceversions/bases/memcached-operator.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/bases/memcached-operator.clusterserviceversion.yaml index 63dabe42f32..f33f7566618 100644 --- a/internal/generate/testdata/clusterserviceversions/bases/memcached-operator.clusterserviceversion.yaml +++ b/internal/generate/testdata/clusterserviceversions/bases/memcached-operator.clusterserviceversion.yaml @@ -24,9 +24,9 @@ spec: deployments: null strategy: "" installModes: - - supported: true + - supported: false type: OwnNamespace - - supported: true + - supported: false type: SingleNamespace - supported: false type: MultiNamespace diff --git a/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml index a8dcf6eff43..c613212c873 100644 --- a/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml +++ b/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml @@ -33,9 +33,9 @@ spec: deployments: null strategy: "" installModes: - - supported: true + - supported: false type: OwnNamespace - - supported: true + - supported: false type: SingleNamespace - supported: false type: MultiNamespace diff --git a/internal/generate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml index af7b345e646..b31e1e5c020 100644 --- a/internal/generate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml +++ b/internal/generate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml @@ -149,9 +149,9 @@ spec: serviceAccountName: default strategy: deployment installModes: - - supported: true + - supported: false type: OwnNamespace - - supported: true + - supported: false type: SingleNamespace - supported: false type: MultiNamespace diff --git a/internal/scorecard/examples/custom-scorecard-tests/bundle/manifests/memcached-operator.clusterserviceversion.yaml b/internal/scorecard/examples/custom-scorecard-tests/bundle/manifests/memcached-operator.clusterserviceversion.yaml index bd7772a602c..2d8b0fd9265 100644 --- a/internal/scorecard/examples/custom-scorecard-tests/bundle/manifests/memcached-operator.clusterserviceversion.yaml +++ b/internal/scorecard/examples/custom-scorecard-tests/bundle/manifests/memcached-operator.clusterserviceversion.yaml @@ -15,7 +15,7 @@ metadata: }, "status": { "nodes": null - } + } } ] capabilities: Basic Install @@ -43,7 +43,7 @@ spec: displayName: Nodes path: nodes x-descriptors: - - 'urn:alm:descriptor:com.tectonic.ui:nodes' + - 'urn:alm:descriptor:com.tectonic.ui:nodes' description: Placeholder description displayName: Memcached Operator icon: @@ -162,9 +162,9 @@ spec: serviceAccountName: memcached-operator strategy: deployment installModes: - - supported: true + - supported: false type: OwnNamespace - - supported: true + - supported: false type: SingleNamespace - supported: false type: MultiNamespace diff --git a/internal/scorecard/testdata/bundle.tar.gz b/internal/scorecard/testdata/bundle.tar.gz index 5f67c14017abcea5aee95567d6c422ee493df1e3..f4c044cd2626f2fb5020041a7aa93d0e4ef9f122 100644 GIT binary patch literal 2746 zcmV;r3PtrFiwFQD5P(xNXnBQ}A>B`^AusREAW4=e;VcAvrfDF)=2QPCfLQq8?Rr zct{8hf>2Qf=oo(jLiiT@O8rQ%cu4+jX(S;(r=NuUmVYDUs9@-5NRG5c+Ue{HKS!U+ zvyeF)f0G$;^>M}@3@I-LPXg*QoR}_Q)(FSpDvB)lnbsnO7pKY<}SGkfViHG+ZmDuS6+7QuI;2F;)uF1EE4ds z2!+SwA#O@*FXE}Ydu+x7ZuCfgLnd5MedtRThl#8{*PMk<0rSjPi@=^-$s?r=E7uC) zF;NRfCh)_jWEM*OGv_xZqw!U`U1M+g)D&CWAlPW#LehcD%&Kz(HhA3VMamqQ`Y=V; zIDP=G0Kp~j93=jfNfOSAWeG=#2QGu{gfK7?;mXbx=J6OrySmk^PQWhmClOVPA&JYV zua>vY4f1oG$Y2P*Gc2ADVSBb>t_lP0+6f;EFIwfq(1WG`bM*?e9xr=K$Wg@OV*c>3rG!GQGng-n)qDTV%g911XVD(6hyW}!Pi{$(S>_dJ>12*A&V>U}f{5?tvHp;>Bc`z@XBDRYzwas+)mTlKJR1?FlTGVm1wu|SY!*=aA zdsxFxYI6RTL12sA-e~HZY`573Udzj-;{F-Nj{kW4{K6mSGdtvrq6RAjTGW^~Sm~O= zG)|%@lnM@BWJV!k=-H+!y?YAsiecN>3HYmEFCKoA2*o#!Rx&9M9tlxYE`WbztEW(U z=?1iA`%u}HsyoZ%TPR}_$rT7epWpx!$#BV`l1~$Iev&bV+mG(0e2-A{|JHo%b8*W4 z>`HIdZy!+Z|G{_m6$(9FsM?Qzj%^-xmd+H4{k;9B zTR`_|=)K5WK$Q;Q3wku!32%-;f7lr3gCZ+8wSq7p|GkR8&SG>V5=ljw1rt;u$R&cx z(gVQ9ZirDT4^bMIOl#@Iv`DhJ1G(MP=d-l)6|rD6zWS+e`dY=o|E)bRhR^{Z@@axKWXFSZiRi?9Zc*tjIQEJX z$vPlFOEVtm6SnPpf5??xt{qJF0=EaV$toyof@n++R)}@&ZPy zpel1M?P6M72p$Cma$wRZZU(1$O))#W=^w7(rd}ujEZXM^HT@;Qa1Z*i$+>PckSR-R z7tbbF0bqS5&Y`q>cd?^{V+%#PUGPnfCNt1TtEjSS&7)>7oWQ7xZ)DwGefUDFuXmm| z`J-MFjJ4i2sanR>?d0Ny+^281N_AZXy^!CP_i85LeWU*tcj){dVIDr#)(G71tTNROB?>B za7Pl1q2w`{vT4F~PgT)*Lz^$NmT4G1T1&^qPrEb?&1cG1Vc>n9;wy}}lky4Vb+NHf zzF4D*z4R=6qR7{q&qcL-T$>GX7V60%MxTDK_RQE3#-%x{9`wE>sFm7LwcNhnWP~PN zY&Szau40Jc6we8-wZtGh8kAwHDL#xI{xLDesBICssWGyqjKW4=ZIf#>3mbilO|FsE zHu_FB`QmpXb%TV!=r?#yWAH0KqEgb;RwJ#xS824;8mj4qS&B;!Wu~sHyiYS0QabX1 za-{CnYRpGhw=HagEUmud^`F<=|LJ#~_WsX4AlHBCozv@2`pvNA{O@$@=YQw)wAX9T z|NDU2ZJpu@Q1)#mA?iPc@)6RbJxu}+4(R!-Z+qzsF`vW1Tdw}3?-RU`Le_TD6pAL) z4lJZ^Aocvrh+jM%4*HLfrqbCBE!dZlaTg#zCd@8$TKoj&G!<-D3O3YSG!aS@dMOD! zS=V$C<2x6ZO7BI=&;-nzIz3xy9pp4RZT|kz?zX-eMf9LG1-{${)K;7lvV~ySQ z-#_bjTmA0?>iSQ!!8fe{x72?216uvQ+R0t;WZh^Kv(^^O`czsFe3d!aTq1R_SWx7-j}|tF%`2qJ9mP1|3Ruu1vknQav^F=R`~F;q!naJFXchgg z&8)-l3geB__shL2z1UjNDKPF0vZ(IfLUjLm825tSn7iiW$Uaqk4rNuX~gx&RjHgMYh?_OY= z{<~fLeA{_oOa0&Nbx&*ezq;-Jf9?s&_rI3eEie3UvGsx;?7+gNVmbBFC>oMr&V#S{ zm9FYP>SS*X%@~}L77ZFPbnJosmo^uF9K68)X&f`D-Vg$|`9FmF`u)$oGid#PAMhgo zZ$18Z3(fwo|LSwnx99|J_5Xg||8+^&`u`rF?EjC67us+{uig26fUQsPfPGkW@J
K} zTmH=8$mtEcgI;gY?G2B*&RO3X9+Cdru~a}3D=G;&0vX8g=k4QmJ^!78pX=XezDwOD z^Q zAXHQVI>w)X5WdB}Qa=(b9+CfA8cE2H=_et-H*gSo4ADU% zM0s-YHOKy%e)~y(x-_CQF1X?-*T2)4yW}c>Wi%b!*oZVZ^{Q)kZ6_TON7RL3k${h7 zC_E;QaeG>i5l^nDQgU5|tq|AZQk5Y7v z<9px=5L^P!LE_JtB;mYRmT;7K;4;`w2m>P#uI*f59*;q^>l@AL6f7iv5>d4rk+_Wd zYI&R7AU~&x3`XEZqvH7pwr4BmsxaWLo$yf6BCqIi7z(^iHjw-=h- z!*GggZe`_ma(OqtzrA~spPj~lzIn#Gz8t$QhT@y2Ny0Cm$8nK0QARQmtP<`Z3x6-eB&ry=m4%x+OO-K| z@Qa9)y=G(wayS)`k7V^Z3%n@g5T>fMF>^p67^qb!5}&z(a=$il#p<)bUFUq^)2PmZ zD_LbIB&JzyH!uX^OaP?@>~)Q4W^RgGK3Rv0HShZKkt#Y`eapni!s|MIBdbyLc)( z?ACs>hc)b^Cg*P%1a`>nm8QPUcAH(`rMzsb^Pgbs^v|bHFZ_N!^NgHP)L?}`iyHG9 zi(XTh#z_=~Qo$LFD4CemO;LLL7~}=Rj-gZVR>8K8o>K@#rQz3!P<&t7VT{l9MC=?!ZAKW8xLwf(2ytMKm0YedDvMwGpOtH z_MdM2+@(?WB5(XuI)Hn0X0pTF0^|FrF{B4Y0&Z#mVeI{975^y<$B{@R6=fDo{Dg>> z2r5et03W*{hNL_mXlQV6cosTNu#(q9NaahjaY)O1&?%ct+&Hu?`& zKGsw;P$P1e1mq;E5UtpIfC4qZUyOi;PS6D<%TX@WhlI|XX!@bY=WAn}XjEJa%}vxOViMNW^5e>-sc&7CV{89Tzbv|!aU-IoNlQahEFoA%p` z(4>pqW~j$i3^APHk>I747-Xk_GVCblBXG-Dy9 z^BpKh>b9)Lymxim!Y;_t>Kk7FdCC2sUU$&m|2YKY`Y*j+dihDe8Frlioo@a7@AUg; z!}k1t2&moGDXsuz-{umc{$nT~AU)c%B=F#Xp1{8B{f7tj#&>@ao}d4n!LZxr|3g6C<=-M2eaC(E@S*!m;KwbZ7Hu$C$;Eww5tT%LW{X1s^tpA3CL0kVF0@BwyTI-24 z?SC5aycv*NMq>q$-Nx=Uy#FzlbPVEMU`GmHj!P;86M45l+n#AHq<6W#By^2r^9EAB zSYe_TP8)e_rTYymfXY*MzO(zr)qyEeH*_p0^4tdt8^q=nQsp{goNxs!=#g261XtRa zo6&uHu0!ElrcShqe%E%^VR(h{#_5OU-j!Z#E$Qmz8)%~NUVF%26TNf`If29OKwE9; zKK1k1ug^X(@47?&3hp=Nn-$(W@cu@ip8p>brNlR`0-s<1_Yu|Be+PhO{o~~d3tw7% z6H9o${s%*+?f)JGcIm&{#n0EB2X@r|T{!gB?tgXR0MOR|hk)|^uT^%-3;#xJv!Dk% zu&}9EPQ5gWMkHA9;A?)RtNJ%N*;_+12B)M&gGP)Tduacy&BgBrd-y+%Vpw3oW$JLJKXl Q&_WIV2gG!XO#n~;0J-^lGXMYp diff --git a/internal/scorecard/testdata/bundle/manifests/memcached-operator.clusterserviceversion.yaml b/internal/scorecard/testdata/bundle/manifests/memcached-operator.clusterserviceversion.yaml index bd7772a602c..2d8b0fd9265 100644 --- a/internal/scorecard/testdata/bundle/manifests/memcached-operator.clusterserviceversion.yaml +++ b/internal/scorecard/testdata/bundle/manifests/memcached-operator.clusterserviceversion.yaml @@ -15,7 +15,7 @@ metadata: }, "status": { "nodes": null - } + } } ] capabilities: Basic Install @@ -43,7 +43,7 @@ spec: displayName: Nodes path: nodes x-descriptors: - - 'urn:alm:descriptor:com.tectonic.ui:nodes' + - 'urn:alm:descriptor:com.tectonic.ui:nodes' description: Placeholder description displayName: Memcached Operator icon: @@ -162,9 +162,9 @@ spec: serviceAccountName: memcached-operator strategy: deployment installModes: - - supported: true + - supported: false type: OwnNamespace - - supported: true + - supported: false type: SingleNamespace - supported: false type: MultiNamespace diff --git a/website/content/en/docs/building-operators/golang/operator-scope.md b/website/content/en/docs/building-operators/golang/operator-scope.md index 0e77bb9c7e0..f87932a730d 100644 --- a/website/content/en/docs/building-operators/golang/operator-scope.md +++ b/website/content/en/docs/building-operators/golang/operator-scope.md @@ -9,23 +9,23 @@ weight: 50 A namespace-scoped operator watches and manages resources in a single Namespace, whereas a cluster-scoped operator watches and manages resources cluster-wide. -An operator should be cluster-scoped if it watches resources that can be created in any Namespace. An operator should -be namespace-scoped if it is intended to be flexibly deployed. This scope permits +An operator should be cluster-scoped if it watches resources that can be created in any Namespace. An operator should +be namespace-scoped if it is intended to be flexibly deployed. This scope permits decoupled upgrades, namespace isolation for failures and monitoring, and differing API definitions. -By default, `operator-sdk init` scaffolds a cluster-scoped operator. This document details conversion of default -operator projects to namespaced-scoped operators. Before proceeding, be aware that your operator may be better suited -as cluster-scoped. For example, the [cert-manager][cert-manager] operator is often deployed with cluster-scoped +By default, `operator-sdk init` scaffolds a cluster-scoped operator. This document details conversion of default +operator projects to namespaced-scoped operators. Before proceeding, be aware that your operator may be better suited +as cluster-scoped. For example, the [cert-manager][cert-manager] operator is often deployed with cluster-scoped permissions and watches so that it can manage and issue certificates for an entire cluster. -**IMPORTANT**: When a [Manager][ctrl-manager] instance is created in the `main.go` file, the -Namespaces are set via [Manager Options][ctrl-options] as described below. These Namespaces should be watched and -cached for the Client which is provided by the Manager.Only clients provided by cluster-scoped Managers are able +**IMPORTANT**: When a [Manager][ctrl-manager] instance is created in the `main.go` file, the +Namespaces are set via [Manager Options][ctrl-options] as described below. These Namespaces should be watched and +cached for the Client which is provided by the Manager.Only clients provided by cluster-scoped Managers are able to manage cluster-scoped CRD's. For further information see: [CRD scope doc][crd-scope-doc]. ## Watching resources in all Namespaces (default) -A [Manager][ctrl-manager] is initialized with no Namespace option specified, or `Namespace: ""` will +A [Manager][ctrl-manager] is initialized with no Namespace option specified, or `Namespace: ""` will watch all Namespaces: ```go @@ -34,7 +34,7 @@ mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, Port: 9443, - LeaderElection: enableLeaderElection, + LeaderElection: enableLeaderElection, LeaderElectionID: "f1c5ece8.example.com", }) ... @@ -42,7 +42,7 @@ mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ ## Watching resources in a single Namespace -To restrict the scope of the [Manager's][ctrl-manager] cache to a specific Namespace set the `Namespace` field +To restrict the scope of the [Manager's][ctrl-manager] cache to a specific Namespace set the `Namespace` field in [Options][ctrl-options]: ```go @@ -53,11 +53,11 @@ mgr, err = ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ MetricsBindAddress: metricsAddr, }) ... -``` +``` ## Watching resources in a set of Namespaces -It is possible to use [`MultiNamespacedCacheBuilder`][multi-namespaced-cache-builder] from +It is possible to use [`MultiNamespacedCacheBuilder`][multi-namespaced-cache-builder] from [Options][ctrl-options] to watch and manage resources in a set of Namespaces: ```go @@ -71,29 +71,29 @@ mgr, err = ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ }) ... ``` -In the above example, a CR created in a Namespace not in the set passed to `Options` will not be reconciled by +In the above example, a CR created in a Namespace not in the set passed to `Options` will not be reconciled by its controller because the [Manager][ctrl-manager] does not manage that Namespace. -**IMPORTANT:** Note that this is not intended to be used for excluding Namespaces, this is better done via a Predicate. +**IMPORTANT:** Note that this is not intended to be used for excluding Namespaces, this is better done via a Predicate. ## Restricting Roles and permissions -An operator's scope defines its [Manager's][ctrl-manager] cache's scope but not the permissions to access the resources. -After updating the Manager's scope to be Namespaced, the cluster's [Role-Based Access Control (RBAC)][k8s-rbac] +An operator's scope defines its [Manager's][ctrl-manager] cache's scope but not the permissions to access the resources. +After updating the Manager's scope to be Namespaced, the cluster's [Role-Based Access Control (RBAC)][k8s-rbac] permissions should be restricted accordingly. -These permissions are found in the directory `config/rbac/`. The `ClusterRole` in `role.yaml` and `ClusterRoleBinding` +These permissions are found in the directory `config/rbac/`. The `ClusterRole` in `role.yaml` and `ClusterRoleBinding` in `role_binding.yaml` are used to grant the operator permissions to access and manage its resources. -**NOTE** For changing the operator's scope only the `role.yaml` and `role_binding.yaml` manifests need to be updated. -For the purposes of this doc, the other RBAC manifests `_editor_role.yaml`, `_viewer_role.yaml`, +**NOTE** For changing the operator's scope only the `role.yaml` and `role_binding.yaml` manifests need to be updated. +For the purposes of this doc, the other RBAC manifests `_editor_role.yaml`, `_viewer_role.yaml`, and `auth_proxy_*.yaml` are not relevant to changing the operator's resource permissions. -### Changing the permissions +### Changing the permissions -To change the scope of the RBAC permissions from cluster-wide to a specific namespace, you will need to use `Role`s +To change the scope of the RBAC permissions from cluster-wide to a specific namespace, you will need to use `Role`s and `RoleBinding`s instead of `ClusterRole`s and `ClusterRoleBinding`s, respectively. - + [`RBAC markers`][rbac-markers] defined in the controller (e.g `controllers/memcached_controller.go`) are used to generate the operator's [RBAC ClusterRole][rbac-clusterrole] (e.g `config/rbac/role.yaml`). The default markers don't specify a `namespace` property and will result in a `ClusterRole`. @@ -124,10 +124,10 @@ metadata: ... ``` -We also need to update our `ClusterRoleBindings` to `RoleBindings` since they are not regenerated: +We also need to update our `ClusterRoleBindings` to `RoleBindings` since they are not regenerated: ```yaml - + apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: @@ -142,12 +142,12 @@ subjects: namespace: system ``` - + -## Using environment variables for Namespace +## Using environment variables for Namespace -Instead of having any Namespaces hard-coded in the `main.go` file a good practice is to use environment +Instead of having any Namespaces hard-coded in the `main.go` file a good practice is to use environment variables to allow the restrictive configurations ### Configuring Namespace scoped operators @@ -161,7 +161,7 @@ func getWatchNamespace() (string, error) { // which specifies the Namespace to watch. // An empty value means the operator is running with cluster scope. var watchNamespaceEnvVar = "WATCH_NAMESPACE" - + ns, found := os.LookupEnv(watchNamespaceEnvVar) if !found { return "", fmt.Errorf("%s must be set", watchNamespaceEnvVar) @@ -170,7 +170,7 @@ func getWatchNamespace() (string, error) { } ``` -- Use the environment variable value: +- Use the environment variable value: ```go ... @@ -186,7 +186,7 @@ mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Port: 9443, LeaderElection: enableLeaderElection, LeaderElectionID: "f1c5ece8.example.com", - Namespace: watchNamespace, // namespaced-scope when the value is not an empty string + Namespace: watchNamespace, // namespaced-scope when the value is not an empty string }) ... ``` @@ -213,11 +213,11 @@ spec: - name: WATCH_NAMESPACE valueFrom: fieldRef: - fieldPath: metadata.namespace + fieldPath: metadata.namespace terminationGracePeriodSeconds: 10 -``` +``` -**NOTE** `WATCH_NAMESPACE` here will always be set as the namespace where the operator is deployed. +**NOTE** `WATCH_NAMESPACE` here will always be set as the namespace where the operator is deployed. ### Configuring cluster-scoped operators with MultiNamespacedCacheBuilder @@ -238,12 +238,12 @@ options := ctrl.Options{ Port: 9443, LeaderElection: enableLeaderElection, LeaderElectionID: "f1c5ece8.example.com", - Namespace: watchNamespace, // namespaced-scope when the value is not an empty string + Namespace: watchNamespace, // namespaced-scope when the value is not an empty string } // Add support for MultiNamespace set in WATCH_NAMESPACE (e.g ns1,ns2) if strings.Contains(watchNamespace, ",") { - setupLog.Infof("manager will be watching namespace %q", watchNamespace) + setupLog.Infof("manager will be watching namespace %q", watchNamespace) // configure cluster-scoped with MultiNamespacedCacheBuilder options.Namespace = "" options.NewCache = cache.MultiNamespacedCacheBuilder(strings.Split(watchNamespace, ",")) @@ -260,7 +260,43 @@ if strings.Contains(watchNamespace, ",") { value: "ns1,ns2" terminationGracePeriodSeconds: 10 ... -``` +``` + +## Updating your CSV's installModes + +If your operator is [integrated with OLM][olm-integration], you will want to update your [CSV base's][csv-base] +`spec.installModes` list to support the desired namespacing requirements. Support for multiple types of namespacing +is allowed, so supporting multiple install modes in a CSV is permitted. After doing so, update your +[bundle][bundle-quickstart] or [package manifests][packagemanifests-quickstart] by following the linked guides. + +## Watching resources in all Namespaces (default) + +Only the `AllNamespaces` install mode is `supported: true` by default, so no changes are required. + +## Watching resources in a single Namespace + +If the operator can watch its own namespace, set the following in your `spec.installModes` list: + +```yaml + - type: OwnNamespace + supported: true +``` + +If the operator can watch a single namespace that is not its own, set the following in your `spec.installModes` list: + +```yaml + - type: SingleNamespace + supported: true +``` + +## Watching resources in multiple Namespaces + +If the operator can watch multiple namespaces, set the following in your `spec.installModes` list: + +```yaml + - type: MultiNamespace + supported: true +``` [cert-manager]: https://github.com/jetstack/cert-manager [ctrl-manager]: https://godoc.org/sigs.k8s.io/controller-runtime/pkg/manager#Manager @@ -271,3 +307,7 @@ if strings.Contains(watchNamespace, ",") { [rbac-clusterrole]: https://kubernetes.io/docs/reference/access-authn-authz/rbac/#role-and-clusterrole [crd-scope-doc]: /docs/building-operators/golang/crds-scope/ [rbac-markers]: https://book.kubebuilder.io/reference/markers/rbac.html +[olm-integration]: /docs/olm-integration +[csv-base]: /docs/olm-integration/generation/#kustomize-files +[bundle]: /docs/olm-integration/quickstart-bundle +[packagemanifests-quickstart]: /docs/olm-integration/quickstart-package-manifests From e1d93419cc7a3345740df5f10e6897a8f610e4f8 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 31 Aug 2020 23:20:01 -0400 Subject: [PATCH 5/9] manifest generation: remove `metadata.namespace` from manifests (#3813) --- .../fragments/gen-bundle-namespaced.yaml | 18 ++++++ .../generate/internal/genutil_suite_test.go | 27 +++++++++ .../generate/internal/manifests.go | 14 +++++ .../generate/internal/manifests_test.go | 56 +++++++++++++++++++ 4 files changed, 115 insertions(+) create mode 100644 changelog/fragments/gen-bundle-namespaced.yaml create mode 100644 internal/cmd/operator-sdk/generate/internal/genutil_suite_test.go create mode 100644 internal/cmd/operator-sdk/generate/internal/manifests_test.go diff --git a/changelog/fragments/gen-bundle-namespaced.yaml b/changelog/fragments/gen-bundle-namespaced.yaml new file mode 100644 index 00000000000..5d366957639 --- /dev/null +++ b/changelog/fragments/gen-bundle-namespaced.yaml @@ -0,0 +1,18 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + When generating bundles and packagemanifests, remove `metadata.namespace` from + namespaced resources when writing them into the `manifests` directory to avoid + validation errors. + + # kind is one of: + # - addition + # - change + # - deprecation + # - removal + # - bugfix + kind: bugfix + + # Is this a breaking change? + breaking: false diff --git a/internal/cmd/operator-sdk/generate/internal/genutil_suite_test.go b/internal/cmd/operator-sdk/generate/internal/genutil_suite_test.go new file mode 100644 index 00000000000..d49609b333c --- /dev/null +++ b/internal/cmd/operator-sdk/generate/internal/genutil_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package genutil + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestGenutil(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Collector Suite") +} diff --git a/internal/cmd/operator-sdk/generate/internal/manifests.go b/internal/cmd/operator-sdk/generate/internal/manifests.go index 8a875506cc9..60996c3a60a 100644 --- a/internal/cmd/operator-sdk/generate/internal/manifests.go +++ b/internal/cmd/operator-sdk/generate/internal/manifests.go @@ -41,5 +41,19 @@ func GetManifestObjects(c *collector.Manifests) (objs []controllerutil.Object) { _, clusterRoleObjs := c.SplitCSVClusterPermissionsObjects() objs = append(objs, clusterRoleObjs...) + removeNamespace(objs) return objs } + +// removeNamespace removes the namespace field of resources intended to be inserted into +// an OLM manifests directory. +// +// This is required to pass OLM validations which require that namespaced resources do +// not include explicit namespace settings. OLM automatically installs namespaced +// resources in the same namespace that the operator is installed in, which is determined +// at runtime, not bundle/packagemanifests creation time. +func removeNamespace(objs []controllerutil.Object) { + for _, obj := range objs { + obj.SetNamespace("") + } +} diff --git a/internal/cmd/operator-sdk/generate/internal/manifests_test.go b/internal/cmd/operator-sdk/generate/internal/manifests_test.go new file mode 100644 index 00000000000..7fc775af83e --- /dev/null +++ b/internal/cmd/operator-sdk/generate/internal/manifests_test.go @@ -0,0 +1,56 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package genutil + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/operator-sdk/internal/generate/collector" +) + +var _ = Describe("GetManifestObjects", func() { + It("should unset the namespace", func() { + m := collector.Manifests{ + Roles: []rbacv1.Role{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "foo"}}, + {ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}}, + }, + ClusterRoles: []rbacv1.ClusterRole{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}}, + }, + ServiceAccounts: []corev1.ServiceAccount{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "foo"}}, + {ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}}, + }, + V1beta1CustomResourceDefinitions: []apiextensionsv1beta1.CustomResourceDefinition{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}}, + }, + V1CustomResourceDefinitions: []apiextensionsv1.CustomResourceDefinition{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}}, + }, + } + objs := GetManifestObjects(&m) + Expect(objs).To(HaveLen(len(m.Roles) + len(m.ClusterRoles) + len(m.ServiceAccounts) + len(m.V1CustomResourceDefinitions) + len(m.V1beta1CustomResourceDefinitions))) + for _, obj := range objs { + Expect(obj.GetNamespace()).To(BeEmpty()) + } + }) +}) From 4cc2eef6de8ec72b76e6e095c62668742bfca467 Mon Sep 17 00:00:00 2001 From: mikeshng <58747157+mikeshng@users.noreply.github.com> Date: Fri, 14 Aug 2020 07:56:19 -0400 Subject: [PATCH 6/9] fix: (helm) - in some rare helm install cases, status is missing InstallSuccessful (#3735) Fixed a bug that caused the Helm operator not to set the `InstallSuccessful` and `UpgradeSuccessful` status reasons when the status update fails during installation and upgrade. Closes #3728 --- changelog/fragments/helm-install-status-fix.yaml | 4 ++++ internal/helm/controller/reconcile.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 changelog/fragments/helm-install-status-fix.yaml diff --git a/changelog/fragments/helm-install-status-fix.yaml b/changelog/fragments/helm-install-status-fix.yaml new file mode 100644 index 00000000000..95d12eef6ef --- /dev/null +++ b/changelog/fragments/helm-install-status-fix.yaml @@ -0,0 +1,4 @@ +entries: + - description: Fixed a bug that caused the Helm operator not to set the `InstallSuccessful` and `UpgradeSuccessful` status reasons when the status update fails during installation and upgrade. + kind: "bugfix" + breaking: false diff --git a/internal/helm/controller/reconcile.go b/internal/helm/controller/reconcile.go index 407c6d6b183..d25629faafd 100644 --- a/internal/helm/controller/reconcile.go +++ b/internal/helm/controller/reconcile.go @@ -316,6 +316,20 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile. } log.Info("Reconciled release") + reason := types.ReasonUpgradeSuccessful + if expectedRelease.Version == 1 { + reason = types.ReasonInstallSuccessful + } + message := "" + if expectedRelease.Info != nil { + message = expectedRelease.Info.Notes + } + status.SetCondition(types.HelmAppCondition{ + Type: types.ConditionDeployed, + Status: types.StatusTrue, + Reason: reason, + Message: message, + }) status.DeployedRelease = &types.HelmAppRelease{ Name: expectedRelease.Name, Manifest: expectedRelease.Manifest, From 281f5c40ba50d3c75b9d89dc91fd3c7b59f95b80 Mon Sep 17 00:00:00 2001 From: Camila Macedo Date: Thu, 27 Aug 2020 17:23:30 +0100 Subject: [PATCH 7/9] Bugfix: Helm operator: add finalizers permission for created APIs (#3779) Co-authored-by: Joe Lanford --- changelog/fragments/helm-roles-permissions.yaml | 16 ++++++++++++++++ .../internal/templates/config/rbac/role.go | 1 + 2 files changed, 17 insertions(+) create mode 100644 changelog/fragments/helm-roles-permissions.yaml diff --git a/changelog/fragments/helm-roles-permissions.yaml b/changelog/fragments/helm-roles-permissions.yaml new file mode 100644 index 00000000000..cab6210c135 --- /dev/null +++ b/changelog/fragments/helm-roles-permissions.yaml @@ -0,0 +1,16 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + In Helm projects, fix operator permissions for Openshift deployments by adding a `/finalizers` rule in the operator's role. + + # kind is one of: + # - addition + # - change + # - deprecation + # - removal + # - bugfix + kind: "bugfix" + + # Is this a breaking change? + breaking: false diff --git a/internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/role.go b/internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/role.go index a285e6b473a..5ceb6a06dc4 100644 --- a/internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/role.go +++ b/internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/role.go @@ -145,6 +145,7 @@ const rulesFragment = ` ## resources: - {{.Resource.Plural}} - {{.Resource.Plural}}/status + - {{.Resource.Plural}}/finalizers verbs: - create - delete From 02567694b5821ae0cf1ba18f6ad48369e423858a Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Mon, 24 Aug 2020 14:26:43 -0400 Subject: [PATCH 8/9] internal/olm/operator: handle conflicting OperatorGroups (#3689) --- .../fragments/operatorgroup-conflict.yaml | 7 ++ internal/olm/operator/operator_suite_test.go | 27 ++++ .../olm/operator/packagemanifests_manager.go | 14 +-- internal/olm/operator/tenancy.go | 98 ++++++++++++--- internal/olm/operator/tenancy_test.go | 118 ++++++++++++++++++ 5 files changed, 240 insertions(+), 24 deletions(-) create mode 100644 changelog/fragments/operatorgroup-conflict.yaml create mode 100644 internal/olm/operator/operator_suite_test.go create mode 100644 internal/olm/operator/tenancy_test.go diff --git a/changelog/fragments/operatorgroup-conflict.yaml b/changelog/fragments/operatorgroup-conflict.yaml new file mode 100644 index 00000000000..6a077e1a222 --- /dev/null +++ b/changelog/fragments/operatorgroup-conflict.yaml @@ -0,0 +1,7 @@ +entries: + - description: > + Prevent `run packagemanifests` from creating an OperatorGroup if one already exists in a namespace, + and use that OperatorGroup if its target namespaces exactly match those passed in `--install-mode`. + See [#3681](https://github.com/operator-framework/operator-sdk/issues/3681). + kind: bugfix + breaking: false diff --git a/internal/olm/operator/operator_suite_test.go b/internal/olm/operator/operator_suite_test.go new file mode 100644 index 00000000000..7315c06185e --- /dev/null +++ b/internal/olm/operator/operator_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package olm + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestOperator(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Operator Suite") +} diff --git a/internal/olm/operator/packagemanifests_manager.go b/internal/olm/operator/packagemanifests_manager.go index 199ac8f3804..38e30eebc10 100644 --- a/internal/olm/operator/packagemanifests_manager.go +++ b/internal/olm/operator/packagemanifests_manager.go @@ -109,9 +109,14 @@ func (m *packageManifestsManager) run(ctx context.Context) (err error) { return fmt.Errorf("an operator with name %q is present and has resource errors\n%s", pkgName, status) } + log.Info("Creating resources") + // Create OperatorGroup first to ensure no conflicts exist in m.namespace. + if err := m.createOperatorGroup(ctx, pkgName); err != nil { + return err + } + // New CatalogSource. catsrc := newCatalogSource(pkgName, m.namespace) - log.Info("Creating catalog source") if err = m.client.DoCreate(ctx, catsrc); err != nil { return fmt.Errorf("error creating catalog source: %w", err) } @@ -132,12 +137,7 @@ func (m *packageManifestsManager) run(ctx context.Context) (err error) { sub := newSubscription(csv.GetName(), m.namespace, withPackageChannel(pkgName, channel), withCatalogSource(getCatalogSourceName(pkgName), m.namespace)) - // New SDK-managed OperatorGroup. - og := newSDKOperatorGroup(m.namespace, - withTargetNamespaces(m.targetNamespaces...)) - objects := []runtime.Object{sub, og} - log.Info("Creating resources") - if err = m.client.DoCreate(ctx, objects...); err != nil { + if err = m.client.DoCreate(ctx, sub); err != nil { return fmt.Errorf("error creating operator resources: %w", err) } diff --git a/internal/olm/operator/tenancy.go b/internal/olm/operator/tenancy.go index 21ed5e650ce..c184bcba43d 100644 --- a/internal/olm/operator/tenancy.go +++ b/internal/olm/operator/tenancy.go @@ -15,30 +15,39 @@ package olm import ( + "context" "fmt" + "reflect" + "sort" "strings" - olmapiv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" + operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + log "github.com/sirupsen/logrus" + "sigs.k8s.io/controller-runtime/pkg/client" + + olmclient "github.com/operator-framework/operator-sdk/internal/olm/client" + "github.com/operator-framework/operator-sdk/internal/operator" ) // Mapping of installMode string values to types, for validation. -var installModeStrings = map[string]olmapiv1alpha1.InstallModeType{ - string(olmapiv1alpha1.InstallModeTypeOwnNamespace): olmapiv1alpha1.InstallModeTypeOwnNamespace, - string(olmapiv1alpha1.InstallModeTypeSingleNamespace): olmapiv1alpha1.InstallModeTypeSingleNamespace, - string(olmapiv1alpha1.InstallModeTypeMultiNamespace): olmapiv1alpha1.InstallModeTypeMultiNamespace, - string(olmapiv1alpha1.InstallModeTypeAllNamespaces): olmapiv1alpha1.InstallModeTypeAllNamespaces, +var installModeStrings = map[string]operatorsv1alpha1.InstallModeType{ + string(operatorsv1alpha1.InstallModeTypeOwnNamespace): operatorsv1alpha1.InstallModeTypeOwnNamespace, + string(operatorsv1alpha1.InstallModeTypeSingleNamespace): operatorsv1alpha1.InstallModeTypeSingleNamespace, + string(operatorsv1alpha1.InstallModeTypeMultiNamespace): operatorsv1alpha1.InstallModeTypeMultiNamespace, + string(operatorsv1alpha1.InstallModeTypeAllNamespaces): operatorsv1alpha1.InstallModeTypeAllNamespaces, } // installModeCompatible ensures installMode is compatible with the namespaces // and CSV's installModes being used. -func installModeCompatible(csv *olmapiv1alpha1.ClusterServiceVersion, installMode olmapiv1alpha1.InstallModeType, +func installModeCompatible(csv *operatorsv1alpha1.ClusterServiceVersion, installMode operatorsv1alpha1.InstallModeType, operatorNamespace string, targetNamespaces []string) error { err := validateInstallModeForNamespaces(installMode, targetNamespaces) if err != nil { return err } - if installMode == olmapiv1alpha1.InstallModeTypeOwnNamespace { + if installMode == operatorsv1alpha1.InstallModeTypeOwnNamespace { if ns := targetNamespaces[0]; ns != operatorNamespace { return fmt.Errorf("installMode %s namespace %q must match namespace %q", installMode, ns, operatorNamespace) @@ -54,13 +63,13 @@ func installModeCompatible(csv *olmapiv1alpha1.ClusterServiceVersion, installMod // parseInstallModeKV parses an installMode string of the format // installModeFormat. -func parseInstallModeKV(raw, operatorNs string) (olmapiv1alpha1.InstallModeType, []string, error) { +func parseInstallModeKV(raw, operatorNs string) (operatorsv1alpha1.InstallModeType, []string, error) { modeSplit := strings.Split(raw, "=") - if allNs := string(olmapiv1alpha1.InstallModeTypeAllNamespaces); raw == allNs || modeSplit[0] == allNs { - return olmapiv1alpha1.InstallModeTypeAllNamespaces, nil, nil + if allNs := string(operatorsv1alpha1.InstallModeTypeAllNamespaces); raw == allNs || modeSplit[0] == allNs { + return operatorsv1alpha1.InstallModeTypeAllNamespaces, nil, nil } - if ownNs := string(olmapiv1alpha1.InstallModeTypeOwnNamespace); raw == ownNs || modeSplit[0] == ownNs { - return olmapiv1alpha1.InstallModeTypeOwnNamespace, []string{operatorNs}, nil + if ownNs := string(operatorsv1alpha1.InstallModeTypeOwnNamespace); raw == ownNs || modeSplit[0] == ownNs { + return operatorsv1alpha1.InstallModeTypeOwnNamespace, []string{operatorNs}, nil } if len(modeSplit) != 2 { return "", nil, fmt.Errorf("installMode string %q is malformatted, must be: %s", raw, installModeFormat) @@ -76,19 +85,19 @@ func parseInstallModeKV(raw, operatorNs string) (olmapiv1alpha1.InstallModeType, } // validateInstallModeForNamespaces ensures namespaces are valid given mode. -func validateInstallModeForNamespaces(mode olmapiv1alpha1.InstallModeType, namespaces []string) error { +func validateInstallModeForNamespaces(mode operatorsv1alpha1.InstallModeType, namespaces []string) error { switch mode { - case olmapiv1alpha1.InstallModeTypeOwnNamespace, olmapiv1alpha1.InstallModeTypeSingleNamespace: + case operatorsv1alpha1.InstallModeTypeOwnNamespace, operatorsv1alpha1.InstallModeTypeSingleNamespace: if len(namespaces) != 1 || namespaces[0] == "" { return fmt.Errorf("installMode %s must be passed with exactly one non-empty namespace, have: %+q", mode, namespaces) } - case olmapiv1alpha1.InstallModeTypeMultiNamespace: + case operatorsv1alpha1.InstallModeTypeMultiNamespace: if len(namespaces) < 2 { return fmt.Errorf("installMode %s must be passed with more than one non-empty namespaces, have: %+q", mode, namespaces) } - case olmapiv1alpha1.InstallModeTypeAllNamespaces: + case operatorsv1alpha1.InstallModeTypeAllNamespaces: if len(namespaces) != 0 && namespaces[0] != "" { return fmt.Errorf("installMode %s must be passed with no namespaces, have: %+q", mode, namespaces) @@ -98,3 +107,58 @@ func validateInstallModeForNamespaces(mode olmapiv1alpha1.InstallModeType, names } return nil } + +// createOperatorGroup creates an OperatorGroup using pkgName if an OperatorGroup does not exist. +// If one exists in the desired namespace and it's target namespaces do not match the desired set, +// createOperatorGroup will return an error. +func (m *packageManifestsManager) createOperatorGroup(ctx context.Context, pkgName string) error { + // Check OperatorGroup existence, since we cannot create a second OperatorGroup in namespace. + og, ogFound, err := getOperatorGroup(ctx, m.client, m.namespace) + if err != nil { + return err + } + if ogFound { + // Simple check for OperatorGroup compatibility: if namespaces are not an exact match, + // the user must manage the resource themselves. + sort.Strings(og.Status.Namespaces) + sort.Strings(m.targetNamespaces) + if !reflect.DeepEqual(og.Status.Namespaces, m.targetNamespaces) { + msg := fmt.Sprintf("namespaces %+q do not match desired namespaces %+q", og.Status.Namespaces, m.targetNamespaces) + if og.GetName() == operator.SDKOperatorGroupName { + return fmt.Errorf("existing SDK-managed operator group's %s, "+ + "please clean up existing operators `operator-sdk cleanup` before running package %q", msg, pkgName) + } + return fmt.Errorf("existing operator group %q's %s, "+ + "please ensure it has the exact namespace set before running package %q", og.GetName(), msg, pkgName) + } + log.Infof(" Using existing operator group %q", og.GetName()) + } else { + // New SDK-managed OperatorGroup. + og = newSDKOperatorGroup(m.namespace, withTargetNamespaces(m.targetNamespaces...)) + if err = m.client.DoCreate(ctx, og); err != nil { + return fmt.Errorf("error creating operator resources: %w", err) + } + } + return nil +} + +// getOperatorGroup returns true if an operator group in namespace was found and that operator group. +// If more than one operator group exists in namespace, this function will return an error +// since CSVs in namespace will have an error status in that case. +func getOperatorGroup(ctx context.Context, c *olmclient.Client, namespace string) (*operatorsv1.OperatorGroup, bool, error) { + ogList := &operatorsv1.OperatorGroupList{} + if err := c.KubeClient.List(ctx, ogList, client.InNamespace(namespace)); err != nil { + return nil, false, err + } + if len(ogList.Items) == 0 { + return nil, false, nil + } + if len(ogList.Items) != 1 { + var names []string + for _, og := range ogList.Items { + names = append(names, og.GetName()) + } + return nil, true, fmt.Errorf("more than one operator group in namespace %s: %+q", namespace, names) + } + return &ogList.Items[0], true, nil +} diff --git a/internal/olm/operator/tenancy_test.go b/internal/olm/operator/tenancy_test.go new file mode 100644 index 00000000000..bd32c8add48 --- /dev/null +++ b/internal/olm/operator/tenancy_test.go @@ -0,0 +1,118 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package olm + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + olmclient "github.com/operator-framework/operator-sdk/internal/olm/client" + "github.com/operator-framework/operator-sdk/internal/operator" +) + +var _ = Describe("Tenancy", func() { + Describe("createOperatorGroup", func() { + var ( + m *packageManifestsManager + ctx context.Context + err error + + packageName = "test-operator" + namespace = "default" + nonSDKOperatorGroupName = "my-og" + ) + + BeforeEach(func() { + m = &packageManifestsManager{ + operatorManager: &operatorManager{ + namespace: namespace, + client: &olmclient.Client{KubeClient: fake.NewFakeClient()}, + }, + } + ctx = context.TODO() + }) + + Context("with no existing OperatorGroup", func() { + It("creates one successfully", func() { + err = m.createOperatorGroup(ctx, packageName) + Expect(err).To(BeNil()) + og, ogExists, err := getOperatorGroup(ctx, m.client, m.namespace) + Expect(err).To(BeNil()) + Expect(ogExists).To(BeTrue()) + Expect(og.GetName()).To(Equal(operator.SDKOperatorGroupName)) + }) + }) + + Context("with an existing, valid OperatorGroup", func() { + It("returns no error and the existing SDK OperatorGroup is unchanged", func() { + existingOG := createOperatorGroupHelper(ctx, m.client.KubeClient, operator.SDKOperatorGroupName, namespace) + err = m.createOperatorGroup(ctx, packageName) + Expect(err).To(BeNil()) + og, ogExists, err := getOperatorGroup(ctx, m.client, m.namespace) + Expect(err).To(BeNil()) + Expect(ogExists).To(BeTrue()) + Expect(og.GetName()).To(Equal(existingOG.GetName())) + }) + It("returns no error and the existing non-SDK OperatorGroup is unchanged", func() { + existingOG := createOperatorGroupHelper(ctx, m.client.KubeClient, nonSDKOperatorGroupName, namespace) + err = m.createOperatorGroup(ctx, packageName) + Expect(err).To(BeNil()) + og, ogExists, err := getOperatorGroup(ctx, m.client, m.namespace) + Expect(err).To(BeNil()) + Expect(ogExists).To(BeTrue()) + Expect(og.GetName()).To(Equal(existingOG.GetName())) + }) + It("returns no error and the existing OperatorGroup in another namespace is unchanged", func() { + otherNS := "my-ns" + existingOG := createOperatorGroupHelper(ctx, m.client.KubeClient, operator.SDKOperatorGroupName, otherNS) + err = m.createOperatorGroup(ctx, packageName) + Expect(err).To(BeNil()) + og, ogExists, err := getOperatorGroup(ctx, m.client, m.namespace) + Expect(err).To(BeNil()) + Expect(ogExists).To(BeTrue()) + Expect(og.GetName()).To(Equal(existingOG.GetName())) + Expect(og.GetNamespace()).NotTo(Equal(existingOG.GetNamespace())) + }) + }) + + Context("with an existing, invalid OperatorGroup", func() { + It("returns an error for an SDK OperatorGroup", func() { + _ = createOperatorGroupHelper(ctx, m.client.KubeClient, operator.SDKOperatorGroupName, namespace, "foo") + err = m.createOperatorGroup(ctx, packageName) + Expect(err.Error()).To(ContainSubstring(`existing SDK-managed operator group's namespaces ["foo"] do not match desired namespaces []`)) + }) + It("returns an error for a non-SDK OperatorGroup", func() { + _ = createOperatorGroupHelper(ctx, m.client.KubeClient, nonSDKOperatorGroupName, namespace, "foo") + err = m.createOperatorGroup(ctx, packageName) + Expect(err.Error()).To(ContainSubstring(`existing operator group "my-og"'s namespaces ["foo"] do not match desired namespaces []`)) + }) + }) + }) + +}) + +func createOperatorGroupHelper(ctx context.Context, c client.Client, name, namespace string, targetNamespaces ...string) (og operatorsv1.OperatorGroup) { + og.SetGroupVersionKind(operatorsv1.SchemeGroupVersion.WithKind("OperatorGroup")) + og.SetName(name) + og.SetNamespace(namespace) + og.Status.Namespaces = targetNamespaces + ExpectWithOffset(1, c.Create(ctx, &og)).Should(Succeed()) + return +} From a5c20045141e172abe0599fe02c66665969390a5 Mon Sep 17 00:00:00 2001 From: Camila Macedo Date: Thu, 10 Sep 2020 12:18:14 +0100 Subject: [PATCH 9/9] bugfix: use the tags in the scorecard scaffolded manifests (#3845) **Description of the change:** - Scorecard manifests created using the image with tags. **Motivation for the change:** Closes: https://github.com/operator-framework/operator-sdk/issues/3840 --- .../fragments/scorecard_scaffolded_version.yaml | 16 ++++++++++++++++ internal/plugins/scorecard/init.go | 11 ++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 changelog/fragments/scorecard_scaffolded_version.yaml diff --git a/changelog/fragments/scorecard_scaffolded_version.yaml b/changelog/fragments/scorecard_scaffolded_version.yaml new file mode 100644 index 00000000000..ca1818f6d9b --- /dev/null +++ b/changelog/fragments/scorecard_scaffolded_version.yaml @@ -0,0 +1,16 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + When scaffolding scorecard configurations, use release versions instead of `latest` in image tags. + + # kind is one of: + # - addition + # - change + # - deprecation + # - removal + # - bugfix + kind: "bugfix" + + # Is this a breaking change? + breaking: false diff --git a/internal/plugins/scorecard/init.go b/internal/plugins/scorecard/init.go index 6eca120ef0f..f128516e3ad 100644 --- a/internal/plugins/scorecard/init.go +++ b/internal/plugins/scorecard/init.go @@ -20,6 +20,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "text/template" "github.com/operator-framework/api/pkg/apis/scorecard/v1alpha3" @@ -29,6 +30,7 @@ import ( "github.com/operator-framework/operator-sdk/internal/plugins/util/kustomize" "github.com/operator-framework/operator-sdk/internal/scorecard" + "github.com/operator-framework/operator-sdk/internal/version" ) const ( @@ -54,17 +56,16 @@ patchesJson6902: ) const ( - // defaultTestImageTag points to the latest-released image. - // TODO: change the tag to "latest" once config scaffolding is in a release, - // as the new config spec won't work with the current latest image. - defaultTestImageTag = "quay.io/operator-framework/scorecard-test:master" - // defaultConfigName is the default scorecard componentconfig's metadata.name, // which must be set on all kustomize-able bases. This name is only used for // `kustomize build` pattern match and not for on-cluster creation. defaultConfigName = "config" ) +// defaultTestImageTag points to the latest-released image. +var defaultTestImageTag = fmt.Sprintf("quay.io/operator-framework/scorecard-test:%s", + strings.TrimSuffix(version.Version, "+git")) + // defaultDir is the default directory in which to generate kustomize bases and the kustomization.yaml. var defaultDir = filepath.Join("config", "scorecard")