Skip to content

Commit e287911

Browse files
theobarberbanytheobarberbany
committed
Fixes due to changes of azure.Config structure
1 parent a1ae7c5 commit e287911

File tree

5 files changed

+58
-54
lines changed

5 files changed

+58
-54
lines changed

‎pkg/cloud/azure/azure.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
"sigs.k8s.io/controller-runtime/pkg/client"
1717

1818
azureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
19-
azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
19+
azureconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
2020

2121
"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/common"
2222
"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
@@ -137,7 +137,7 @@ func CloudConfigTransformer(source string, infra *configv1.Infrastructure, netwo
137137
return "", fmt.Errorf("invalid platform, expected CloudName to be %s", configv1.AzurePublicCloud)
138138
}
139139

140-
var cfg azure.Config
140+
var cfg azureconfig.Config
141141
if err := json.Unmarshal([]byte(source), &cfg); err != nil {
142142
return "", fmt.Errorf("failed to unmarshal the cloud.conf: %w", err)
143143
}

‎pkg/cloud/azure/azure_test.go

+40-38
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,13 @@ import (
88
. "github.com/onsi/gomega"
99
"github.com/onsi/gomega/format"
1010
configv1 "github.com/openshift/api/config/v1"
11+
"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
1112
"github.com/stretchr/testify/assert"
12-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13-
azureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
14-
azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
1513

14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1615
"sigs.k8s.io/cloud-provider-azure/pkg/azclient"
17-
ratelimitconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
18-
19-
"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
16+
azureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
17+
azconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
2018
)
2119

2220
const (
@@ -126,7 +124,7 @@ func makeInfrastructureResource(platform configv1.PlatformType, cloudName config
126124
}
127125

128126
// makeExpectedConfig sets some repetitive default fields for tests, assuming that they are not already set.
129-
func makeExpectedConfig(config *azure.Config, cloud configv1.AzureCloudEnvironment) azure.Config {
127+
func makeExpectedConfig(config *azconfig.Config, cloud configv1.AzureCloudEnvironment) azconfig.Config {
130128
if config.ClusterServiceLoadBalancerHealthProbeMode == "" {
131129
config.ClusterServiceLoadBalancerHealthProbeMode = azureconsts.ClusterServiceLoadBalancerHealthProbeModeShared
132130
}
@@ -135,7 +133,7 @@ func makeExpectedConfig(config *azure.Config, cloud configv1.AzureCloudEnvironme
135133
config.VMType = "standard"
136134
}
137135

138-
config.AzureAuthConfig = ratelimitconfig.AzureAuthConfig{
136+
config.AzureClientConfig = azconfig.AzureClientConfig{
139137
ARMClientConfig: azclient.ARMClientConfig{
140138
Cloud: string(cloud),
141139
},
@@ -153,95 +151,99 @@ func makeExpectedConfig(config *azure.Config, cloud configv1.AzureCloudEnvironme
153151
func TestCloudConfigTransformer(t *testing.T) {
154152
tc := []struct {
155153
name string
156-
source azure.Config
157-
expected azure.Config
154+
source azconfig.Config
155+
expected azconfig.Config
158156
infra *configv1.Infrastructure
159157
errMsg string
160158
}{
161159
{
162160
name: "Non Azure returns an error",
163-
source: azure.Config{},
161+
source: azconfig.Config{},
164162
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureStackCloud),
165163
errMsg: fmt.Sprintf("invalid platform, expected CloudName to be %s", configv1.AzurePublicCloud),
166164
},
167165
{
168166
name: "Azure sets the vmType to standard and cloud to AzurePublicCloud when neither is set",
169-
source: azure.Config{},
170-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzurePublicCloud),
167+
source: azconfig.Config{},
168+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzurePublicCloud),
171169
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
172170
},
173171
{
174172
name: "Azure doesn't modify vmType if user set",
175-
source: azure.Config{VMType: "vmss"},
176-
expected: makeExpectedConfig(&azure.Config{VMType: "vmss"}, configv1.AzurePublicCloud),
173+
source: azconfig.Config{VMType: "vmss"},
174+
expected: makeExpectedConfig(&azconfig.Config{VMType: "vmss"}, configv1.AzurePublicCloud),
177175
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
178176
},
179177
{
180178
name: "Azure sets the cloud to AzurePublicCloud and keeps existing fields",
181-
source: azure.Config{
179+
source: azconfig.Config{
182180
ResourceGroup: "test-rg",
183181
},
184-
expected: makeExpectedConfig(&azure.Config{ResourceGroup: "test-rg"}, configv1.AzurePublicCloud),
182+
expected: makeExpectedConfig(&azconfig.Config{ResourceGroup: "test-rg"}, configv1.AzurePublicCloud),
185183
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
186184
},
187185
{
188186
name: "Azure keeps the cloud set to AzurePublicCloud",
189-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzurePublicCloud)}}},
190-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzurePublicCloud),
187+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzurePublicCloud)}}},
188+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzurePublicCloud),
191189
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
192190
},
193191
{
194192
name: "Azure keeps the cloud set to US Gov cloud",
195-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzureUSGovernmentCloud)}}},
196-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzureUSGovernmentCloud),
193+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzureUSGovernmentCloud)}}},
194+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzureUSGovernmentCloud),
197195
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureUSGovernmentCloud),
198196
},
199197
{
200198
name: "Azure keeps the cloud set to China cloud",
201-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzureChinaCloud)}}},
202-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzureChinaCloud),
199+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzureChinaCloud)}}},
200+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzureChinaCloud),
203201
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureChinaCloud),
204202
},
205203
{
206204
name: "Azure keeps the cloud set to German cloud",
207-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzureGermanCloud)}}},
208-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzureGermanCloud),
205+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzureGermanCloud)}}},
206+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzureGermanCloud),
209207
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureGermanCloud),
210208
},
211209
{
212210
name: "Azure throws an error if the infra has an invalid cloud",
213-
source: azure.Config{},
211+
source: azconfig.Config{},
214212
infra: makeInfrastructureResource(configv1.AzurePlatformType, "AzureAnotherCloud"),
215213
errMsg: "status.platformStatus.azure.cloudName: Unsupported value: \"AzureAnotherCloud\": supported values: \"AzureChinaCloud\", \"AzureGermanCloud\", \"AzurePublicCloud\", \"AzureStackCloud\", \"AzureUSGovernmentCloud\"",
216214
},
217215
{
218216
name: "Azure keeps the cloud set in the source when there is not one set in infrastructure",
219-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzurePublicCloud)}}},
220-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzurePublicCloud),
217+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzurePublicCloud)}}},
218+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzurePublicCloud),
221219
infra: makeInfrastructureResource(configv1.AzurePlatformType, ""),
222220
},
223221
{
224222
name: "Azure sets the cloud to match the infrastructure if an empty string is provided in source",
225-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: ""}}},
226-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzurePublicCloud),
223+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: ""}}},
224+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzurePublicCloud),
227225
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
228226
},
229227
{
230228
name: "Azure sets the cloud to match the infrastructure if an empty string is provided in source and the infrastructure is non standard",
231-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: ""}}},
232-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzureUSGovernmentCloud),
229+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: ""}}},
230+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzureUSGovernmentCloud),
233231
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureUSGovernmentCloud),
234232
},
235233
{
236-
name: "Azure returns an error if the source config conflicts with the infrastructure",
237-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: string(configv1.AzurePublicCloud)}}},
234+
name: "Azure returns an error if the source config conflicts with the infrastructure",
235+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{
236+
Cloud: string(configv1.AzurePublicCloud)}},
237+
},
238238
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureUSGovernmentCloud),
239239
errMsg: "invalid user-provided cloud.conf: \\\"cloud\\\" field in user-provided\n\t\t\t\tcloud.conf conflicts with infrastructure object",
240240
},
241241
{
242-
name: "Azure keeps the cloud set to AzurePublicCloud if the source is upper case",
243-
source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{ARMClientConfig: azclient.ARMClientConfig{Cloud: "AZUREPUBLICCLOUD"}}},
244-
expected: makeExpectedConfig(&azure.Config{}, configv1.AzurePublicCloud),
242+
name: "Azure keeps the cloud set to AzurePublicCloud if the source is upper case",
243+
source: azconfig.Config{AzureClientConfig: azconfig.AzureClientConfig{ARMClientConfig: azclient.ARMClientConfig{
244+
Cloud: "AZUREPUBLICCLOUD"}},
245+
},
246+
expected: makeExpectedConfig(&azconfig.Config{}, configv1.AzurePublicCloud),
245247
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
246248
},
247249
}
@@ -262,7 +264,7 @@ func TestCloudConfigTransformer(t *testing.T) {
262264
g.Expect(err).Should(MatchError(tc.errMsg))
263265
g.Expect(actual).Should(Equal(""))
264266
} else {
265-
var observed azure.Config
267+
var observed azconfig.Config
266268
g.Expect(json.Unmarshal([]byte(actual), &observed)).To(Succeed(), "Unmarshal of observed data should succeed")
267269
g.Expect(observed).Should(Equal(tc.expected))
268270
}

‎pkg/cloud/azurestack/azurestack.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
configv1 "github.com/openshift/api/config/v1"
1010
appsv1 "k8s.io/api/apps/v1"
1111
azureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
12-
azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
12+
azureconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
1313
"sigs.k8s.io/controller-runtime/pkg/client"
1414

1515
"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/common"
@@ -106,7 +106,7 @@ func CloudConfigTransformer(source string, infra *configv1.Infrastructure, netwo
106106
return "", fmt.Errorf("invalid platform, expected CloudName to be %s", configv1.AzureStackCloud)
107107
}
108108

109-
var cfg azure.Config
109+
var cfg azureconfig.Config
110110
if err := json.Unmarshal([]byte(source), &cfg); err != nil {
111111
return "", fmt.Errorf("failed to unmarshal the cloud.conf: %w", err)
112112
}

‎pkg/cloud/azurestack/azurestack_test.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
. "github.com/onsi/gomega"
99
configv1 "github.com/openshift/api/config/v1"
1010
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11-
azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
11+
azconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
1212

1313
"github.com/stretchr/testify/assert"
1414

@@ -112,26 +112,26 @@ func makeInfrastructureResource(platform configv1.PlatformType, cloudName config
112112
func TestCloudConfigTransformer(t *testing.T) {
113113
tc := []struct {
114114
name string
115-
source azure.Config
116-
expected azure.Config
115+
source azconfig.Config
116+
expected azconfig.Config
117117
infra *configv1.Infrastructure
118118
errMsg string
119119
}{
120120
{
121121
name: "Azure Stack Hub sets the vmType to standard",
122-
source: azure.Config{},
123-
expected: azure.Config{VMType: "standard"},
122+
source: azconfig.Config{},
123+
expected: azconfig.Config{VMType: "standard"},
124124
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureStackCloud),
125125
},
126126
{
127127
name: "Azure Stack Hub doesn't modify vmType if user set",
128-
source: azure.Config{VMType: "vmss"},
129-
expected: azure.Config{VMType: "vmss"},
128+
source: azconfig.Config{VMType: "vmss"},
129+
expected: azconfig.Config{VMType: "vmss"},
130130
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureStackCloud),
131131
},
132132
{
133133
name: "Non Azure Stack Hub returns an error",
134-
source: azure.Config{},
134+
source: azconfig.Config{},
135135
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
136136
errMsg: fmt.Sprintf("invalid platform, expected CloudName to be %s", configv1.AzureStackCloud),
137137
},
@@ -149,7 +149,7 @@ func TestCloudConfigTransformer(t *testing.T) {
149149
g.Expect(err).Should(MatchError(tc.errMsg))
150150
g.Expect(actual).Should(Equal(""))
151151
} else {
152-
var observed azure.Config
152+
var observed azconfig.Config
153153
g.Expect(json.Unmarshal([]byte(actual), &observed)).To(Succeed(), "Unmarshal of observed data should succeed")
154154

155155
g.Expect(observed).Should(Equal(tc.expected))

‎pkg/controllers/cloud_config_sync_controller_test.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package controllers
22

33
import (
44
"context"
5+
"fmt"
56
"time"
67

78
. "github.com/onsi/ginkgo/v2"
@@ -27,7 +28,7 @@ const (
2728
infraCloudConfName = "test-config"
2829
infraCloudConfKey = "foo"
2930

30-
defaultAzureConfig = `{"cloud":"AzurePublicCloud","tenantId":"0000000-0000-0000-0000-000000000000","subscriptionId":"0000000-0000-0000-0000-000000000000","vmType":"standard","putVMSSVMBatchSize":0,"enableMigrateToIPBasedBackendPoolAPI":false,"clusterServiceLoadBalancerHealthProbeMode":"shared"}`
31+
defaultAzureConfig = `{"cloud":"AzurePublicCloud","tenantId":"0000000-0000-0000-0000-000000000000","Entries":null,"subscriptionId":"0000000-0000-0000-0000-000000000000","vmType":"standard","putVMSSVMBatchSize":0,"enableMigrateToIPBasedBackendPoolAPI":false,"clusterServiceLoadBalancerHealthProbeMode":"shared"}`
3132
)
3233

3334
func makeInfrastructureResource(platform configv1.PlatformType) *configv1.Infrastructure {
@@ -279,12 +280,13 @@ var _ = Describe("Cloud config sync controller", func() {
279280
syncedCloudConfigMap := &corev1.ConfigMap{}
280281
err := cl.Get(ctx, syncedConfigMapKey, syncedCloudConfigMap)
281282
g.Expect(err).NotTo(HaveOccurred())
283+
fmt.Printf("\n\nsynced data: \n%s\n", syncedCloudConfigMap.Data[defaultConfigKey])
282284
return syncedCloudConfigMap.Data[defaultConfigKey]
283285
}).Should(Equal(defaultAzureConfig))
284286
})
285287

286288
It("config should be synced up if managed cloud config changed", func() {
287-
changedConfigString := `{"cloud":"AzurePublicCloud","tenantId":"0000000-1234-1234-0000-000000000000","subscriptionId":"0000000-0000-0000-0000-000000000000","vmType":"standard","putVMSSVMBatchSize":0,"enableMigrateToIPBasedBackendPoolAPI":false,"clusterServiceLoadBalancerHealthProbeMode":"shared"}`
289+
changedConfigString := `{"cloud":"AzurePublicCloud","tenantId":"0000000-1234-1234-0000-000000000000","Entries":null,"subscriptionId":"0000000-0000-0000-0000-000000000000","vmType":"standard","putVMSSVMBatchSize":0,"enableMigrateToIPBasedBackendPoolAPI":false,"clusterServiceLoadBalancerHealthProbeMode":"shared"}`
288290
changedManagedConfig := managedCloudConfig.DeepCopy()
289291
changedManagedConfig.Data = map[string]string{"cloud.conf": changedConfigString}
290292
Expect(cl.Update(ctx, changedManagedConfig)).To(Succeed())
@@ -339,7 +341,7 @@ var _ = Describe("Cloud config sync controller", func() {
339341
Expect(cl.Delete(ctx, managedCloudConfig)).Should(Succeed())
340342
managedCloudConfig = nil
341343

342-
changedInfraConfigString := `{"cloud":"AzurePublicCloud","tenantId":"0000000-1234-1234-0000-000000000000","subscriptionId":"0000000-0000-0000-0000-000000000000","vmType":"standard","putVMSSVMBatchSize":0,"enableMigrateToIPBasedBackendPoolAPI":false,"clusterServiceLoadBalancerHealthProbeMode":"shared"}`
344+
changedInfraConfigString := `{"cloud":"AzurePublicCloud","tenantId":"0000000-1234-1234-0000-000000000000","Entries":null,"subscriptionId":"0000000-0000-0000-0000-000000000000","vmType":"standard","putVMSSVMBatchSize":0,"enableMigrateToIPBasedBackendPoolAPI":false,"clusterServiceLoadBalancerHealthProbeMode":"shared"}`
343345
changedInfraConfig := infraCloudConfig.DeepCopy()
344346
changedInfraConfig.Data = map[string]string{infraCloudConfKey: changedInfraConfigString}
345347
Expect(cl.Update(ctx, changedInfraConfig)).Should(Succeed())

0 commit comments

Comments
 (0)