Skip to content

Commit 0b389c1

Browse files
theobarberbanytheobarberbany
committed
Fixes due to changes of azure.Config structure
1 parent 65f5859 commit 0b389c1

File tree

5 files changed

+53
-55
lines changed

5 files changed

+53
-55
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"
@@ -139,7 +139,7 @@ func CloudConfigTransformer(source string, infra *configv1.Infrastructure, netwo
139139
return "", fmt.Errorf("invalid platform, expected CloudName to be %s", configv1.AzurePublicCloud)
140140
}
141141

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

‎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

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

33
import (
44
"context"
5-
"time"
65

76
. "github.com/onsi/ginkgo/v2"
87
. "github.com/onsi/gomega"
@@ -13,7 +12,6 @@ import (
1312
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1413
"k8s.io/client-go/kubernetes/scheme"
1514
"k8s.io/client-go/tools/record"
16-
clocktesting "k8s.io/utils/clock/testing"
1715
"k8s.io/utils/ptr"
1816
ctrl "sigs.k8s.io/controller-runtime"
1917
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -204,7 +202,6 @@ var _ = Describe("Cloud config sync controller", func() {
204202
ClusterOperatorStatusClient: ClusterOperatorStatusClient{
205203
Client: cl,
206204
Recorder: rec,
207-
Clock: clocktesting.NewFakePassiveClock(time.Now()),
208205
ManagedNamespace: targetNamespaceName,
209206
},
210207
Scheme: scheme.Scheme,
@@ -404,7 +401,6 @@ var _ = Describe("Cloud config sync reconciler", func() {
404401
BeforeEach(func() {
405402
reconciler = &CloudConfigReconciler{
406403
ClusterOperatorStatusClient: ClusterOperatorStatusClient{
407-
Clock: clocktesting.NewFakePassiveClock(time.Now()),
408404
Client: cl,
409405
ManagedNamespace: targetNamespaceName,
410406
},

0 commit comments

Comments
 (0)