Skip to content

Commit cbe3f11

Browse files
atiratreeatiratree
committed
cleanup flag validation after they have been added to the installer
1 parent 56d4c7e commit cbe3f11

File tree

2 files changed

+85
-26
lines changed

2 files changed

+85
-26
lines changed

‎pkg/cmd/render/render.go‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,9 @@ func (r *renderOpts) Validate() error {
113113
return err
114114
}
115115

116-
// TODO: enable check when the installer is updated
117-
//if len(r.manifest.OperatorImage) == 0 {
118-
// return errors.New("missing required flag: --manifest-operator-image")
119-
//}
116+
if len(r.manifest.OperatorImage) == 0 {
117+
return errors.New("missing required flag: --manifest-operator-image")
118+
}
120119
if len(r.lockHostPath) == 0 {
121120
return errors.New("missing required flag: --manifest-lock-host-path")
122121
}
@@ -131,14 +130,12 @@ func (r *renderOpts) Validate() error {
131130
return err
132131
}
133132

134-
// TODO add after installer PR is merged
135-
//if len(r.operandKubernetesVersion) == 0 {
136-
// return errors.New("missing operand kubernetes version: --operand-kubernetes-version")
137-
//}
138-
//if _, err := semver.Parse(r.operandKubernetesVersion); err != nil {
139-
// return err
140-
//}
141-
133+
if len(r.operandKubernetesVersion) == 0 {
134+
return errors.New("missing operand kubernetes version: --operand-kubernetes-version")
135+
}
136+
if _, err := semver.Parse(r.operandKubernetesVersion); err != nil {
137+
return fmt.Errorf("could not parse --operand-kubernetes-version: %v", err)
138+
}
142139
return nil
143140
}
144141

@@ -151,10 +148,6 @@ func (r *renderOpts) Complete() error {
151148
return err
152149
}
153150
if r.groupVersionsByFeatureGate == nil {
154-
// TODO remove after installer PR is merged
155-
if len(r.operandKubernetesVersion) == 0 {
156-
r.operandKubernetesVersion = "1.30.0"
157-
}
158151
var err error
159152
r.groupVersionsByFeatureGate, err = apienablement.GetDefaultGroupVersionByFeatureGate(semver.MustParse(r.operandKubernetesVersion))
160153
if err != nil {

‎pkg/cmd/render/render_test.go‎

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,8 @@ func TestRenderCommand(t *testing.T) {
249249
"--config-output-file=",
250250
"--payload-version=test",
251251
"--rendered-manifest-files=" + defaultFGDir,
252+
"--manifest-operator-image=kube-apiserver-operator:latest",
253+
"--operand-kubernetes-version=1.31.0",
252254
},
253255
overrides: []func(*renderOpts){
254256
func(opts *renderOpts) {
@@ -307,6 +309,8 @@ func TestRenderCommand(t *testing.T) {
307309
"--config-output-file=",
308310
"--payload-version=test",
309311
"--rendered-manifest-files=" + defaultFGDir,
312+
"--manifest-operator-image=kube-apiserver-operator:latest",
313+
"--operand-kubernetes-version=1.31.0",
310314
},
311315
setupFunction: func() error {
312316
return ioutil.WriteFile(filepath.Join(assetsInputDir, "config-v6.yaml"), []byte(networkConfigV6), 0644)
@@ -331,6 +335,8 @@ func TestRenderCommand(t *testing.T) {
331335
"--config-output-file=",
332336
"--payload-version=test",
333337
"--rendered-manifest-files=" + defaultFGDir,
338+
"--manifest-operator-image=kube-apiserver-operator:latest",
339+
"--operand-kubernetes-version=1.31.0",
334340
},
335341
setupFunction: func() error {
336342
return ioutil.WriteFile(filepath.Join(assetsInputDir, "config-dual.yaml"), []byte(networkConfigDual), 0644)
@@ -358,6 +364,8 @@ func TestRenderCommand(t *testing.T) {
358364
"--config-output-file=",
359365
"--payload-version=test",
360366
"--rendered-manifest-files=" + defaultFGDir,
367+
"--manifest-operator-image=kube-apiserver-operator:latest",
368+
"--operand-kubernetes-version=1.31.0",
361369
},
362370
testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
363371
issuer := cfg.APIServerArguments["service-account-issuer"]
@@ -378,6 +386,8 @@ func TestRenderCommand(t *testing.T) {
378386
"--config-output-file=",
379387
"--payload-version=test",
380388
"--rendered-manifest-files=" + defaultFGDir,
389+
"--manifest-operator-image=kube-apiserver-operator:latest",
390+
"--operand-kubernetes-version=1.31.0",
381391
},
382392
setupFunction: func() error {
383393
data := ``
@@ -402,6 +412,8 @@ func TestRenderCommand(t *testing.T) {
402412
"--config-output-file=",
403413
"--payload-version=test",
404414
"--rendered-manifest-files=" + defaultFGDir,
415+
"--manifest-operator-image=kube-apiserver-operator:latest",
416+
"--operand-kubernetes-version=1.31.0",
405417
},
406418
setupFunction: func() error {
407419
data := `apiVersion: config.openshift.io/v1
@@ -430,6 +442,8 @@ spec: {}`
430442
"--config-output-file=",
431443
"--payload-version=test",
432444
"--rendered-manifest-files=" + defaultFGDir,
445+
"--manifest-operator-image=kube-apiserver-operator:latest",
446+
"--operand-kubernetes-version=1.31.0",
433447
},
434448
setupFunction: func() error {
435449
data := `apiVersion: config.openshift.io/v1
@@ -459,6 +473,8 @@ spec:
459473
"--config-output-file=",
460474
"--payload-version=test",
461475
"--rendered-manifest-files=" + defaultFGDir,
476+
"--manifest-operator-image=kube-apiserver-operator:latest",
477+
"--operand-kubernetes-version=1.31.0",
462478
},
463479
},
464480
{
@@ -470,6 +486,8 @@ spec:
470486
"--config-output-file=",
471487
"--payload-version=test",
472488
"--rendered-manifest-files=" + defaultFGDir,
489+
"--manifest-operator-image=kube-apiserver-operator:latest",
490+
"--operand-kubernetes-version=1.31.0",
473491
},
474492
setupFunction: func() error {
475493
data := `DUMMY DATA`
@@ -509,6 +527,8 @@ spec:
509527
"--config-output-file=",
510528
"--payload-version=test",
511529
"--rendered-manifest-files=" + defaultFGDir,
530+
"--manifest-operator-image=kube-apiserver-operator:latest",
531+
"--operand-kubernetes-version=1.31.0",
512532
},
513533
testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
514534
if len(cfg.APIServerArguments["shutdown-delay-duration"]) == 0 {
@@ -539,6 +559,8 @@ spec:
539559
"--infra-config-file=" + filepath.Join(assetsInputDir, "infrastructure.yaml"),
540560
"--payload-version=test",
541561
"--rendered-manifest-files=" + defaultFGDir,
562+
"--manifest-operator-image=kube-apiserver-operator:latest",
563+
"--operand-kubernetes-version=1.31.0",
542564
},
543565
setupFunction: func() error {
544566
return ioutil.WriteFile(filepath.Join(assetsInputDir, "infrastructure.yaml"), []byte(infrastructureHA), 0644)
@@ -572,6 +594,8 @@ spec:
572594
"--infra-config-file=" + filepath.Join(assetsInputDir, "infrastructure.yaml"),
573595
"--payload-version=test",
574596
"--rendered-manifest-files=" + defaultFGDir,
597+
"--manifest-operator-image=kube-apiserver-operator:latest",
598+
"--operand-kubernetes-version=1.31.0",
575599
},
576600
setupFunction: func() error {
577601
return ioutil.WriteFile(filepath.Join(assetsInputDir, "infrastructure.yaml"), []byte(infrastructureSNO), 0644)
@@ -735,11 +759,12 @@ func Test_renderOpts_Validate(t *testing.T) {
735759
templateDir := filepath.Join("..", "..", "..", "bindata", "bootkube")
736760

737761
tests := []struct {
738-
name string
739-
assetInputDir string
740-
setupFunction func() error
741-
testFunction func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error
742-
wantErr bool
762+
name string
763+
assetInputDir string
764+
setupFunction func() error
765+
operatorImage string
766+
operandKubernetesVersion string
767+
wantErr bool
743768
}{
744769
{
745770
name: "user provided bound-sa-signing-key only no public part",
@@ -751,7 +776,9 @@ func Test_renderOpts_Validate(t *testing.T) {
751776
}
752777
return ioutil.WriteFile(filepath.Join(assetsInputDir, "0", "bound-service-account-signing-key.key"), []byte(data), 0600)
753778
},
754-
wantErr: true,
779+
operatorImage: "kube-apiserver-operator:latest",
780+
operandKubernetesVersion: "1.31.0",
781+
wantErr: true,
755782
},
756783
{
757784
name: "user provided bound-sa-signing-key only public part",
@@ -763,7 +790,9 @@ func Test_renderOpts_Validate(t *testing.T) {
763790
}
764791
return ioutil.WriteFile(filepath.Join(assetsInputDir, "1", "bound-service-account-signing-key.pub"), []byte(data), 0644)
765792
},
766-
wantErr: true,
793+
operatorImage: "kube-apiserver-operator:latest",
794+
operandKubernetesVersion: "1.31.0",
795+
wantErr: true,
767796
},
768797
{
769798
name: "user provided bound-sa-signing-key - both keys exist",
@@ -778,13 +807,47 @@ func Test_renderOpts_Validate(t *testing.T) {
778807
}
779808
return ioutil.WriteFile(filepath.Join(assetsInputDir, "2", "bound-service-account-signing-key.key"), []byte(data), 0600)
780809
},
810+
operatorImage: "kube-apiserver-operator:latest",
811+
operandKubernetesVersion: "1.31.0",
781812
},
782813
{
783814
name: "user provided bound-sa-signing-key - neither key exists",
784815
assetInputDir: filepath.Join(assetsInputDir, "3"),
785816
setupFunction: func() error {
786817
return os.Mkdir(filepath.Join(assetsInputDir, "3"), 0700)
787818
},
819+
operatorImage: "kube-apiserver-operator:latest",
820+
operandKubernetesVersion: "1.31.0",
821+
},
822+
{
823+
name: "missing --manifest-operator-image flag",
824+
assetInputDir: filepath.Join(assetsInputDir, "4"),
825+
setupFunction: func() error {
826+
return os.Mkdir(filepath.Join(assetsInputDir, "4"), 0700)
827+
},
828+
operatorImage: "",
829+
operandKubernetesVersion: "1.31.0",
830+
wantErr: true,
831+
},
832+
{
833+
name: "missing --operand-kubernetes-version flag",
834+
assetInputDir: filepath.Join(assetsInputDir, "5"),
835+
setupFunction: func() error {
836+
return os.Mkdir(filepath.Join(assetsInputDir, "5"), 0700)
837+
},
838+
operatorImage: "kube-apiserver-operator:latest",
839+
operandKubernetesVersion: "",
840+
wantErr: true,
841+
},
842+
{
843+
name: "invalid --operand-kubernetes-version flag",
844+
assetInputDir: filepath.Join(assetsInputDir, "6"),
845+
setupFunction: func() error {
846+
return os.Mkdir(filepath.Join(assetsInputDir, "6"), 0700)
847+
},
848+
operatorImage: "kube-apiserver-operator:latest",
849+
operandKubernetesVersion: "5",
850+
wantErr: true,
788851
},
789852
}
790853
for _, tt := range tests {
@@ -804,16 +867,19 @@ func Test_renderOpts_Validate(t *testing.T) {
804867
generic: *genericrenderoptions.NewGenericOptions(),
805868
manifest: *genericrenderoptions.NewManifestOptions("kube-apiserver", "openshift/origin-hyperkube:latest"),
806869

807-
lockHostPath: "/var/run/kubernetes/lock",
808-
etcdServerURLs: []string{"https://127.0.0.1:2379"},
809-
etcdServingCA: "root-ca.crt",
870+
lockHostPath: "/var/run/kubernetes/lock",
871+
etcdServerURLs: []string{"https://127.0.0.1:2379"},
872+
etcdServingCA: "root-ca.crt",
873+
operandKubernetesVersion: tt.operandKubernetesVersion,
810874
}
811875
r.generic.TemplatesDir = templateDir
812876

813877
r.generic.AssetInputDir = tt.assetInputDir
814878
r.generic.AssetOutputDir = filepath.Join(outputDir, "manifests")
815879
r.generic.ConfigOutputFile = filepath.Join(outputDir, "configs", "config.yaml")
816880

881+
r.manifest.OperatorImage = tt.operatorImage
882+
817883
if err := r.Validate(); (err != nil) != tt.wantErr {
818884
t.Errorf("renderOpts.Validate() error = %v, wantErr %v", err, tt.wantErr)
819885
}

0 commit comments

Comments
 (0)