Skip to content

Commit 2c922b7

Browse files
elmikoelmiko
committed
add a cloud config transformer for azure stack
This change creates a CloudConfigTransformer for Azure Stack cloud provider. The transformer will unmarshal json from the cloud config into a `Config` struct from the azure cloud provider and then update the the `VMType` field if it is unset. This is being added because Azure has updated the default vm type to value that cannot be added to the basic load balancers which are associated with Azure Stack. The change also adds a test suite to accompany the transformer.
1 parent 4779872 commit 2c922b7

3 files changed

Lines changed: 142 additions & 6 deletions

File tree

‎pkg/cloud/azurestack/azurestack.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@ package azurestack
22

33
import (
44
"embed"
5+
"encoding/json"
56
"fmt"
67

78
"github.com/asaskevich/govalidator"
9+
configv1 "github.com/openshift/api/config/v1"
810
appsv1 "k8s.io/api/apps/v1"
11+
azureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
12+
azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
913
"sigs.k8s.io/controller-runtime/pkg/client"
1014

1115
"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/common"
@@ -86,3 +90,38 @@ func NewProviderAssets(config config.OperatorConfig) (common.CloudProviderAssets
8690
}
8791
return assets, nil
8892
}
93+
94+
// IsAzureStackHub inspects the infrastructure config returns true if it is configured for ASH.
95+
func IsAzureStackHub(platformStatus *configv1.PlatformStatus) bool {
96+
return platformStatus.Azure != nil && platformStatus.Azure.CloudName == configv1.AzureStackCloud
97+
}
98+
99+
// CloudConfigTransformer implements the cloudConfigTransformer. It takes
100+
// the user-provided, legacy cloud provider-compatible configuration and
101+
// modifies it to be compatible with the external cloud provider. It returns
102+
// an error if the platform is not OpenStackPlatformType or if any errors are
103+
// encountered while attempting to rework the configuration.
104+
func CloudConfigTransformer(source string, infra *configv1.Infrastructure, network *configv1.Network) (string, error) {
105+
if !IsAzureStackHub(infra.Status.PlatformStatus) {
106+
return "", fmt.Errorf("invalid platform, expected CloudName to be %s", configv1.AzureStackCloud)
107+
}
108+
109+
var cfg azure.Config
110+
if err := json.Unmarshal([]byte(source), &cfg); err != nil {
111+
return "", fmt.Errorf("failed to unmarshal the cloud.conf: %w", err)
112+
}
113+
114+
// If the virtual machine type is not set we need to make sure it uses
115+
// the "standard" instance type. This is to mitigate an issue in the 1.27
116+
// release where the default instance type was changed to VMSS.
117+
// see OCPBUGS-20213 for more information.
118+
if cfg.VMType == "" {
119+
cfg.VMType = azureconsts.VMTypeStandard
120+
}
121+
122+
cfgbytes, err := json.Marshal(cfg)
123+
if err != nil {
124+
return "", fmt.Errorf("failed to marshal the cloud.conf: %w", err)
125+
}
126+
return string(cfgbytes), nil
127+
}

‎pkg/cloud/azurestack/azurestack_test.go‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,25 @@
11
package azurestack
22

33
import (
4+
"encoding/json"
5+
"fmt"
46
"testing"
57

8+
. "github.com/onsi/gomega"
69
configv1 "github.com/openshift/api/config/v1"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
712

813
"github.com/stretchr/testify/assert"
914

1015
"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
1116
)
1217

18+
const (
19+
infraCloudConfName = "test-config"
20+
infraCloudConfKey = "foo"
21+
)
22+
1323
func TestResourcesRenderingSmoke(t *testing.T) {
1424

1525
tc := []struct {
@@ -62,3 +72,88 @@ func TestResourcesRenderingSmoke(t *testing.T) {
6272
})
6373
}
6474
}
75+
76+
func makeInfrastructureResource(platform configv1.PlatformType, cloudName configv1.AzureCloudEnvironment) *configv1.Infrastructure {
77+
cfg := configv1.Infrastructure{
78+
ObjectMeta: metav1.ObjectMeta{
79+
Name: "cluster",
80+
},
81+
Status: configv1.InfrastructureStatus{
82+
PlatformStatus: &configv1.PlatformStatus{
83+
Type: platform,
84+
},
85+
},
86+
Spec: configv1.InfrastructureSpec{
87+
CloudConfig: configv1.ConfigMapFileReference{
88+
Name: infraCloudConfName,
89+
Key: infraCloudConfKey,
90+
},
91+
PlatformSpec: configv1.PlatformSpec{
92+
Type: platform,
93+
},
94+
},
95+
}
96+
97+
if platform == configv1.AzurePlatformType {
98+
cfg.Status.PlatformStatus.Azure = &configv1.AzurePlatformStatus{
99+
CloudName: cloudName,
100+
}
101+
}
102+
103+
return &cfg
104+
}
105+
106+
// This test is a little complicated with all the JSON marshalling and
107+
// unmarshalling, but it is necessary due to the nature of how this data
108+
// is stored in Kuberenetes. The ConfigMaps containing the cloud config
109+
// will have string encoded JSON objects in them, due to the non-deterministic
110+
// natue of map object in Go we will need to examine the data instead of
111+
// comparing strings.
112+
func TestCloudConfigTransformer(t *testing.T) {
113+
tc := []struct {
114+
name string
115+
source azure.Config
116+
expected azure.Config
117+
infra *configv1.Infrastructure
118+
errMsg string
119+
}{
120+
{
121+
name: "Azure Stack Hub sets the vmType to standard",
122+
source: azure.Config{},
123+
expected: azure.Config{VMType: "standard"},
124+
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureStackCloud),
125+
},
126+
{
127+
name: "Azure Stack Hub doesn't modify vmType if user set",
128+
source: azure.Config{VMType: "vmss"},
129+
expected: azure.Config{VMType: "vmss"},
130+
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureStackCloud),
131+
},
132+
{
133+
name: "Non Azure Stack Hub returns an error",
134+
source: azure.Config{},
135+
infra: makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
136+
errMsg: fmt.Sprintf("invalid platform, expected CloudName to be %s", configv1.AzureStackCloud),
137+
},
138+
}
139+
140+
for _, tc := range tc {
141+
t.Run(tc.name, func(t *testing.T) {
142+
g := NewWithT(t)
143+
144+
src, err := json.Marshal(tc.source)
145+
g.Expect(err).NotTo(HaveOccurred(), "Marshal of source data should succeed")
146+
147+
actual, err := CloudConfigTransformer(string(src), tc.infra, nil)
148+
if tc.errMsg != "" {
149+
g.Expect(err).Should(MatchError(tc.errMsg))
150+
g.Expect(actual).Should(Equal(""))
151+
} else {
152+
var observed azure.Config
153+
g.Expect(json.Unmarshal([]byte(actual), &observed)).To(Succeed(), "Unmarshal of observed data should succeed")
154+
155+
g.Expect(observed).Should(Equal(tc.expected))
156+
}
157+
})
158+
}
159+
}

‎pkg/cloud/cloud.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,14 @@ func GetCloudConfigTransformer(platformStatus *configv1.PlatformStatus) (cloudCo
4545
case configv1.AzurePlatformType:
4646
// We intentionally return nil rather than NoOpTransformer since we
4747
// want to handle this differently in the caller.
48+
// Except on Azure Stack Hub, where we need to lookup the cloud config
49+
// from the managed namespace and also return a config transformer.
4850
// FIXME: We need to implement a transformer for this. Currently we're
49-
// relying on CCO to do the heavy lifting for us.
51+
// relying on CCO to do the heavy lifting for us. The Azure Stack Hub
52+
// transformer is only to fix OCPBUGS-20213.
53+
if azurestack.IsAzureStackHub(platformStatus) {
54+
return azurestack.CloudConfigTransformer, true, nil
55+
}
5056
return nil, true, nil
5157
case configv1.GCPPlatformType:
5258
return common.NoOpTransformer, false, nil
@@ -112,7 +118,7 @@ func getAssetsConstructor(platformStatus *configv1.PlatformStatus) (assetsConstr
112118
case configv1.AWSPlatformType:
113119
return aws.NewProviderAssets, nil
114120
case configv1.AzurePlatformType:
115-
if isAzureStackHub(platformStatus) {
121+
if azurestack.IsAzureStackHub(platformStatus) {
116122
return azurestack.NewProviderAssets, nil
117123
}
118124
return azure.NewProviderAssets, nil
@@ -132,7 +138,3 @@ func getAssetsConstructor(platformStatus *configv1.PlatformStatus) (assetsConstr
132138
return nil, newPlatformNotFoundError(platformStatus.Type)
133139
}
134140
}
135-
136-
func isAzureStackHub(platformStatus *configv1.PlatformStatus) bool {
137-
return platformStatus.Azure != nil && platformStatus.Azure.CloudName == configv1.AzureStackCloud
138-
}

0 commit comments

Comments
 (0)