Skip to content

Commit 261f43d

Browse files
werdes72werdes72
committed
Pass hub and tag to reconciler
1 parent b25a895 commit 261f43d

File tree

10 files changed

+101
-38
lines changed

10 files changed

+101
-38
lines changed

‎controllers/istio_controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,9 @@ func (r *IstioReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
115115
return r.terminateReconciliation(ctx, &istioCR, describederrors.NewDescribedError(imgErr, "Unable to get Istio images environments"),
116116
operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileFailed))
117117
}
118-
hub, imgErr := istioImages.GetHub()
118+
hubAndTag, imgErr := istioImages.GetHubAndImageTag()
119119
if imgErr != nil {
120-
return r.terminateReconciliation(ctx, &istioCR, describederrors.NewDescribedError(imgErr, "Unable to get Istio images hub"),
120+
return r.terminateReconciliation(ctx, &istioCR, describederrors.NewDescribedError(imgErr, "Unable to get Istio images hubAndTag"),
121121
operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileFailed))
122122
}
123123

@@ -172,7 +172,7 @@ func (r *IstioReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
172172
}
173173
}
174174

175-
istioImageVersion, installationErr := r.istioInstallation.Reconcile(ctx, &istioCR, r.statusHandler, hub)
175+
istioImageVersion, installationErr := r.istioInstallation.Reconcile(ctx, &istioCR, r.statusHandler, hubAndTag)
176176
if installationErr != nil {
177177
return r.requeueReconciliation(ctx, &istioCR, installationErr,
178178
operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonIstioInstallUninstallFailed),

‎internal/images/env.go‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ const kymaFipsModeEnabledEnv = "KYMA_FIPS_MODE_ENABLED"
1212

1313
type Image string
1414

15+
type HubTag struct {
16+
Hub string
17+
Tag string
18+
}
19+
1520
func (i Image) GetHub() (string, error) {
1621
if i == "" {
1722
return "", fmt.Errorf("image can not be empty")
@@ -25,6 +30,19 @@ func (i Image) GetHub() (string, error) {
2530
return strings.Join(parts[:len(parts)-1], "/"), nil
2631
}
2732

33+
func (i Image) GetTag() (string, error) {
34+
if i == "" {
35+
return "", fmt.Errorf("image can not be empty")
36+
}
37+
38+
parts := strings.Split(string(i), ":")
39+
if len(parts) != 2 {
40+
return "", fmt.Errorf("image %s does not contain a valid tag", i)
41+
}
42+
43+
return strings.Join(parts[len(parts)-1:], "/"), nil
44+
}
45+
2846
type Images struct {
2947
Pilot Image `env:"pilot,notEmpty"`
3048
InstallCNI Image `env:"install-cni,notEmpty"`
@@ -57,23 +75,36 @@ func GetImages() (*Images, error) {
5775
return &environments, nil
5876
}
5977

60-
func (e *Images) GetHub() (string, error) {
78+
func (e *Images) GetHubAndImageTag() (HubTag, error) {
6179
environments := []Image{e.Pilot, e.InstallCNI, e.ProxyV2}
6280

6381
initialHub, err := environments[0].GetHub()
6482
if err != nil {
65-
return "", fmt.Errorf("failed to get hub for image %s: %w", environments[0], err)
83+
return HubTag{}, fmt.Errorf("failed to get hub for image %s: %w", environments[0], err)
6684
}
67-
// Ensure that all required images are from the same hub
85+
initialTag, err := environments[0].GetTag()
86+
if err != nil {
87+
return HubTag{}, fmt.Errorf("failed to get tag for image %s: %w", environments[0], err)
88+
}
89+
90+
// Ensure that all required images are from the same hub and have the same version tag
6891
for _, image := range environments {
6992
currentHub, err := image.GetHub()
7093
if err != nil {
71-
return "", fmt.Errorf("failed to get hub for image %s: %w", image, err)
94+
return HubTag{}, fmt.Errorf("failed to get hub for image %s: %w", image, err)
7295
}
73-
7496
if currentHub != initialHub {
75-
return "", fmt.Errorf("image %s is not from the same hub as %s", image, initialHub)
97+
return HubTag{}, fmt.Errorf("image %s is not from the same hub as %s", image, environments[0])
98+
}
99+
100+
currentTag, err := image.GetTag()
101+
if err != nil {
102+
return HubTag{}, fmt.Errorf("failed to get tag for image %s: %w", image, err)
103+
}
104+
if currentTag != initialTag {
105+
return HubTag{}, fmt.Errorf("image %s does not have the same tag as %s", image, environments[0])
76106
}
77107
}
78-
return initialHub, nil
108+
109+
return HubTag{Hub: initialHub, Tag: initialTag}, nil
79110
}

‎internal/images/env_test.go‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,27 @@ func TestEnvs(t *testing.T) {
1717
RunSpecs(t, "Environment Suite")
1818
}
1919

20-
var _ = Describe("Images.GetHub", func() {
20+
var _ = Describe("Images.GetHubAndImageTag", func() {
2121
type fields struct {
2222
Pilot images.Image
2323
InstallCNI images.Image
2424
ProxyV2 images.Image
2525
Ztunnel images.Image
2626
}
2727

28-
DescribeTable("GetHub",
29-
func(f fields, want string, wantErr bool, err error) {
28+
DescribeTable("GetHubAndImageTag",
29+
func(f fields, want images.HubTag, wantErr bool, expErr error) {
3030
e := &images.Images{
3131
Pilot: f.Pilot,
3232
InstallCNI: f.InstallCNI,
3333
ProxyV2: f.ProxyV2,
3434
Ztunnel: f.Ztunnel,
3535
}
36-
got, err := e.GetHub()
36+
got, err := e.GetHubAndImageTag()
3737
if wantErr {
3838
Expect(err).To(HaveOccurred())
3939
Expect(err.Error()).To(ContainSubstring("image"))
40+
Expect(err.Error()).To(ContainSubstring(expErr.Error()))
4041
} else {
4142
Expect(err).NotTo(HaveOccurred())
4243
Expect(got).To(Equal(want))
@@ -49,39 +50,61 @@ var _ = Describe("Images.GetHub", func() {
4950
ProxyV2: "docker.io/istio/proxyv2:1.10.0",
5051
Ztunnel: "docker.io/istio/ztunnel:1.10.0",
5152
},
52-
"docker.io/istio",
53+
images.HubTag{Hub: "docker.io/istio", Tag: "1.10.0"},
5354
false,
5455
nil,
5556
),
56-
Entry("invalid image format",
57+
Entry("invalid image hub",
5758
fields{
5859
Pilot: "pilot:1.10.0",
5960
InstallCNI: "docker.io/istio/cni:1.10.0",
6061
ProxyV2: "docker.io/istio/proxyv2:1.10.0",
6162
Ztunnel: "docker.io/istio/ztunnel:1.10.0",
6263
},
63-
"",
64+
images.HubTag{},
6465
true,
6566
fmt.Errorf("image pilot:1.10.0 does not contain a valid hub URL"),
6667
),
68+
Entry("missing image tag",
69+
fields{
70+
Pilot: "docker.io/istio/pilot1.10.0",
71+
InstallCNI: "docker.io/istio/cni:1.10.0",
72+
ProxyV2: "docker.io/istio/proxyv2:1.10.0",
73+
Ztunnel: "docker.io/istio/ztunnel:1.10.0",
74+
},
75+
images.HubTag{},
76+
true,
77+
fmt.Errorf("image docker.io/istio/pilot1.10.0 does not contain a valid tag"),
78+
),
6779
Entry("images from different hubs",
6880
fields{
6981
Pilot: "docker.io/istio/pilot:1.10.0",
7082
InstallCNI: "docker.io/istio/cni:1.10.0",
7183
ProxyV2: "foo.bar/istio/proxyv2:1.10.0",
7284
Ztunnel: "docker.io/istio/ztunnel:1.10.0",
7385
},
74-
"",
86+
images.HubTag{},
7587
true,
7688
fmt.Errorf("image foo.bar/istio/proxyv2:1.10.0 is not from the same hub as docker.io/istio/pilot:1.10.0"),
7789
),
90+
Entry("images with different tags",
91+
fields{
92+
Pilot: "docker.io/istio/pilot:1.10.0",
93+
InstallCNI: "docker.io/istio/cni:1.10.0",
94+
ProxyV2: "docker.io/istio/proxyv2:1.11.0",
95+
Ztunnel: "docker.io/istio/ztunnel:1.10.0",
96+
},
97+
images.HubTag{},
98+
true,
99+
fmt.Errorf("image docker.io/istio/proxyv2:1.11.0 does not have the same tag as docker.io/istio/pilot:1.10.0"),
100+
),
78101
Entry("empty image",
79102
fields{
80103
Pilot: "",
81104
InstallCNI: "docker.io/istio/cni:1.10.0",
82105
ProxyV2: "docker.io/istio/proxyv2:1.10.0",
83106
},
84-
"",
107+
images.HubTag{},
85108
true,
86109
fmt.Errorf("image can not be empty"),
87110
),

‎internal/images/merge.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99

1010
const pullSecretEnvVar = "SKR_IMG_PULL_SECRET"
1111

12-
// MergeHubConfiguration merges the Istio hub configuration to the provided manifest.
13-
func MergeHubConfiguration(manifest []byte, istioImagesHub string) ([]byte, error) {
12+
// MergeHubTagConfiguration merges the Istio hub and tag configuration to the provided manifest.
13+
func MergeHubTagConfiguration(manifest []byte, istioImagesHubTag HubTag) ([]byte, error) {
1414
var templateMap map[string]interface{}
1515
err := yaml.Unmarshal(manifest, &templateMap)
1616
if err != nil {
@@ -19,7 +19,8 @@ func MergeHubConfiguration(manifest []byte, istioImagesHub string) ([]byte, erro
1919

2020
err = mergo.Merge(&templateMap, map[string]interface{}{
2121
"spec": map[string]interface{}{
22-
"hub": istioImagesHub,
22+
"hub": istioImagesHubTag.Hub,
23+
"tag": istioImagesHubTag.Tag,
2324
},
2425
}, mergo.WithOverride)
2526
if err != nil {

‎internal/images/merge_test.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ import (
1313

1414
var _ = Describe("Images merging", func() {
1515

16-
Describe("MergeHubConfiguration", func() {
16+
Describe("MergeHubTagConfiguration", func() {
1717

1818
DescribeTable("merges hub correctly",
19-
func(input string, hub string, expectedHub string, expectsError bool) {
20-
out, err := images.MergeHubConfiguration([]byte(input), hub)
19+
func(input string, hubTag images.HubTag, expectedHub string, expectedTag string, expectsError bool) {
20+
out, err := images.MergeHubTagConfiguration([]byte(input), hubTag)
2121

2222
if expectsError {
2323
Expect(err).To(HaveOccurred())
@@ -31,31 +31,36 @@ var _ = Describe("Images merging", func() {
3131

3232
spec := parsed["spec"].(map[string]interface{})
3333
Expect(spec["hub"]).To(Equal(expectedHub))
34+
Expect(spec["tag"]).To(Equal(expectedTag))
3435
},
3536

3637
Entry("adds hub when missing",
3738
`
3839
spec:
3940
profile: default
4041
`,
42+
images.HubTag{Hub: "my-hub", Tag: "my-tag"},
4143
"my-hub",
42-
"my-hub",
44+
"my-tag",
4345
false,
4446
),
4547

4648
Entry("overrides existing hub",
4749
`
4850
spec:
4951
hub: old-hub
52+
tag: old-tag
5053
`,
54+
images.HubTag{Hub: "new-hub", Tag: "new-tag"},
5155
"new-hub",
52-
"new-hub",
56+
"new-tag",
5357
false,
5458
),
5559

5660
Entry("fails on invalid yaml",
5761
`::: bad yaml :::`,
58-
"hub",
62+
images.HubTag{},
63+
"",
5964
"",
6065
true,
6166
),

‎internal/istiooperator/istiooperator.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77

88
"github.com/coreos/go-semver/semver"
9+
"github.com/kyma-project/istio/operator/internal/images"
910
iopv1alpha1 "istio.io/istio/operator/pkg/apis"
1011
"sigs.k8s.io/yaml"
1112

@@ -52,7 +53,7 @@ func (i *IstioImageVersion) Empty() bool {
5253
}
5354

5455
type Merger interface {
55-
Merge(clusterSize clusterconfig.ClusterSize, istioCR *operatorv1alpha2.Istio, overrides clusterconfig.ClusterConfiguration, istioImagesHub string) (string, error)
56+
Merge(clusterSize clusterconfig.ClusterSize, istioCR *operatorv1alpha2.Istio, overrides clusterconfig.ClusterConfiguration, istioImagesHubTag images.HubTag) (string, error)
5657
GetIstioOperator(clusterSize clusterconfig.ClusterSize) (iopv1alpha1.IstioOperator, error)
5758
GetIstioImageVersion() (IstioImageVersion, error)
5859
}

‎internal/istiooperator/merge.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/kyma-project/istio/operator/internal/images"
1212
)
1313

14-
func (m *IstioMerger) Merge(clusterSize clusterconfig.ClusterSize, istioCR *operatorv1alpha2.Istio, overrides clusterconfig.ClusterConfiguration, istioImagesHub string) (string, error) {
14+
func (m *IstioMerger) Merge(clusterSize clusterconfig.ClusterSize, istioCR *operatorv1alpha2.Istio, overrides clusterconfig.ClusterConfiguration, istioImagesHubTag images.HubTag) (string, error) {
1515
toBeInstalledIop, err := m.GetIstioOperator(clusterSize)
1616
if err != nil {
1717
return "", err
@@ -20,7 +20,7 @@ func (m *IstioMerger) Merge(clusterSize clusterconfig.ClusterSize, istioCR *oper
2020
if err != nil {
2121
return "", err
2222
}
23-
manifestWithOverrideImagesHub, err := images.MergeHubConfiguration(mergedManifest, istioImagesHub)
23+
manifestWithOverrideImagesHub, err := images.MergeHubTagConfiguration(mergedManifest, istioImagesHubTag)
2424
if err != nil {
2525
return "", err
2626
}

‎internal/istiooperator/merge_experimental.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (m *IstioMerger) Merge(clusterSize clusterconfig.ClusterSize, istioCR *oper
3030
return "", err
3131
}
3232

33-
manifestWithOverrideImagesHub, err := images.MergeHubConfiguration(mergedManifest, istioImagesHub)
33+
manifestWithOverrideImagesHub, err := images.MergeHubTagConfiguration(mergedManifest, istioImagesHub)
3434
if err != nil {
3535
return "", err
3636
}

‎internal/reconciliations/istio/install.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package istio
33
import (
44
"context"
55

6+
"github.com/kyma-project/istio/operator/internal/images"
67
"github.com/kyma-project/istio/operator/pkg/lib/gatherer"
78

89
ctrl "sigs.k8s.io/controller-runtime"
@@ -25,7 +26,7 @@ type installArgs struct {
2526
istioOperatorMerger istiooperator.Merger
2627
istioImageVersion istiooperator.IstioImageVersion
2728
istioClient libraryClient
28-
istioImagesHub string
29+
istioImagesHubTag images.HubTag
2930
}
3031

3132
//nolint:funlen // Function 'installIstio' has too many statements (51 > 50) TODO: refactor.
@@ -36,7 +37,7 @@ func installIstio(ctx context.Context, args installArgs) (istiooperator.IstioIma
3637
statusHandler := args.statusHandler
3738
iopMerger := args.istioOperatorMerger
3839
istioClient := args.istioClient
39-
istioImagesHub := args.istioImagesHub
40+
istioImagesHubTag := args.istioImagesHubTag
4041

4142
ctrl.Log.Info("Starting Istio install", "istio version", istioImageVersion.Version())
4243

@@ -80,7 +81,7 @@ func installIstio(ctx context.Context, args installArgs) (istiooperator.IstioIma
8081

8182
ctrl.Log.Info("Installing Istio with", "profile", clusterSize.String())
8283

83-
mergedIstioOperatorPath, err := iopMerger.Merge(clusterSize, istioCR, clusterConfiguration, istioImagesHub)
84+
mergedIstioOperatorPath, err := iopMerger.Merge(clusterSize, istioCR, clusterConfiguration, istioImagesHubTag)
8485
if err != nil {
8586
statusHandler.SetCondition(istioCR, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonCustomResourceMisconfigured))
8687
return istioImageVersion, describederrors.NewDescribedError(err, "Could not merge Istio operator configuration").SetCondition(false)

‎internal/reconciliations/istio/reconciliation.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package istio
33
import (
44
"context"
55

6+
"github.com/kyma-project/istio/operator/internal/images"
67
ctrl "sigs.k8s.io/controller-runtime"
78
"sigs.k8s.io/controller-runtime/pkg/client"
89

@@ -13,7 +14,7 @@ import (
1314
)
1415

1516
type InstallationReconciliation interface {
16-
Reconcile(ctx context.Context, istioCR *operatorv1alpha2.Istio, statusHandler status.Status, istioImageHub string) (istiooperator.IstioImageVersion, describederrors.DescribedError)
17+
Reconcile(ctx context.Context, istioCR *operatorv1alpha2.Istio, statusHandler status.Status, istioImageHub images.HubTag) (istiooperator.IstioImageVersion, describederrors.DescribedError)
1718
}
1819

1920
type Installation struct {
@@ -27,7 +28,7 @@ func (i *Installation) Reconcile(
2728
ctx context.Context,
2829
istioCR *operatorv1alpha2.Istio,
2930
statusHandler status.Status,
30-
istioImagesHub string,
31+
istioImagesHubTag images.HubTag,
3132
) (istiooperator.IstioImageVersion, describederrors.DescribedError) {
3233
istioImageVersion, err := i.Merger.GetIstioImageVersion()
3334
if err != nil {
@@ -43,7 +44,7 @@ func (i *Installation) Reconcile(
4344
istioOperatorMerger: i.Merger,
4445
istioImageVersion: istioImageVersion,
4546
istioClient: i.IstioClient,
46-
istioImagesHub: istioImagesHub,
47+
istioImagesHubTag: istioImagesHubTag,
4748
}
4849
return installIstio(ctx, args)
4950
}

0 commit comments

Comments
 (0)