Skip to content

Revendor library-go and openshift/api for Azure Stack Hub and Azure support #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 2 commits into from
Jul 12, 2021
Merged

Revendor library-go and openshift/api for Azure Stack Hub and Azure support #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6aec7f
Revendor with library-go change for --cloud-provider=external
Jul 2, 2021
1768110
Update to use new IsCloudProviderExternal signature
Jul 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ go 1.16
require (
github.com/onsi/ginkgo v1.16.4
github.com/onsi/gomega v1.13.0
github.com/openshift/api v0.0.0-20210521075222-e273a339932a
github.com/openshift/library-go v0.0.0-20210608075825-51ddcf37b1f8
github.com/openshift/api v0.0.0-20210706092853-b63d499a70ce
github.com/openshift/library-go v0.0.0-20210708191609-4b9033d00d37
github.com/stretchr/testify v1.7.0
k8s.io/api v0.21.1
k8s.io/apiextensions-apiserver v0.21.1
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,10 +413,14 @@ github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQ
github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
github.com/openshift/api v0.0.0-20210521075222-e273a339932a h1:aBPwLqCg66SbQd+HrjB1GhgTfPtqSY4aeB022tEYmE0=
github.com/openshift/api v0.0.0-20210521075222-e273a339932a/go.mod h1:izBmoXbUu3z5kUa4FjZhvekTsyzIWiOoaIgJiZBBMQs=
github.com/openshift/api v0.0.0-20210706092853-b63d499a70ce h1:TI9mXBgwq/coqN9NV85CbF1JOpXhvDSSAn33xdNw+zg=
github.com/openshift/api v0.0.0-20210706092853-b63d499a70ce/go.mod h1:izBmoXbUu3z5kUa4FjZhvekTsyzIWiOoaIgJiZBBMQs=
github.com/openshift/build-machinery-go v0.0.0-20210423112049-9415d7ebd33e/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/client-go v0.0.0-20210521082421-73d9475a9142/go.mod h1:fjS8r9mqDVsPb5td3NehsNOAWa4uiFkYEfVZioQ2gH0=
github.com/openshift/library-go v0.0.0-20210608075825-51ddcf37b1f8 h1:j7mMVstcFACXDXeJB+pLVO3Kuf067+IvFwkw9l/S7Tw=
github.com/openshift/library-go v0.0.0-20210608075825-51ddcf37b1f8/go.mod h1:D0QEmUeYyWP9ORJ92EprPOMkiFg72yX8GM9KxajnMAI=
github.com/openshift/library-go v0.0.0-20210708191609-4b9033d00d37 h1:hr0fJ1QM+JCGnAC/gIbBX5HEGSpm+Gi1rb/+Yq9QWwk=
github.com/openshift/library-go v0.0.0-20210708191609-4b9033d00d37/go.mod h1:rln3LbFNOpENSvhmsfH7g/hqc58IF78+o96yAAp5mq0=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
Expand Down
10 changes: 8 additions & 2 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,13 @@ func getCloudNodeManagerFromImages(platform configv1.PlatformType, images images
}

// ComposeConfig creates a Config for operator
func ComposeConfig(platform configv1.PlatformType, imagesFile, managedNamespace string, isSingleReplica bool) (OperatorConfig, error) {
func ComposeConfig(infrastructure *configv1.Infrastructure, imagesFile, managedNamespace string) (OperatorConfig, error) {
platform, err := GetProviderFromInfrastructure(infrastructure)
if err != nil {
klog.Errorf("Unable to get platform from infrastructure: %s", err)
return OperatorConfig{}, err
}

images, err := getImagesFromJSONFile(imagesFile)
if err != nil {
klog.Errorf("Unable to decode images file from location %s", imagesFile, err)
Expand All @@ -90,7 +96,7 @@ func ComposeConfig(platform configv1.PlatformType, imagesFile, managedNamespace
ManagedNamespace: managedNamespace,
ControllerImage: getCloudControllerManagerFromImages(platform, images),
CloudNodeImage: getCloudNodeManagerFromImages(platform, images),
IsSingleReplica: isSingleReplica,
IsSingleReplica: infrastructure.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode,
}

return config, nil
Expand Down
76 changes: 61 additions & 15 deletions pkg/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,17 +218,22 @@ func TestComposeConfig(t *testing.T) {
defaultManagementNamespace := "test-namespace"

tc := []struct {
name string
namespace string
platform configv1.PlatformType
imagesContent string
expectConfig OperatorConfig
expectError string
isSingleReplica bool
name string
namespace string
infra *configv1.Infrastructure
imagesContent string
expectConfig OperatorConfig
expectError string
}{{
name: "Unmarshal images from file for AWS",
namespace: defaultManagementNamespace,
platform: configv1.AWSPlatformType,
infra: &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.AWSPlatformType,
},
},
},
imagesContent: `{
"cloudControllerManagerAWS": "registry.ci.openshift.org/openshift:aws-cloud-controller-manager",
"cloudControllerManagerOpenStack": "registry.ci.openshift.org/openshift:openstack-cloud-controller-manager"
Expand All @@ -241,7 +246,13 @@ func TestComposeConfig(t *testing.T) {
}, {
name: "Unmarshal images from file for OpenStack",
namespace: defaultManagementNamespace,
platform: configv1.OpenStackPlatformType,
infra: &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.OpenStackPlatformType,
},
},
},
imagesContent: `{
"cloudControllerManagerAWS": "registry.ci.openshift.org/openshift:aws-cloud-controller-manager",
"cloudControllerManagerOpenStack": "registry.ci.openshift.org/openshift:openstack-cloud-controller-manager"
Expand All @@ -254,7 +265,13 @@ func TestComposeConfig(t *testing.T) {
}, {
name: "Unmarshal images from file for unknown platform returns nothing",
namespace: "otherNamespace",
platform: configv1.NonePlatformType,
infra: &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.NonePlatformType,
},
},
},
imagesContent: `{
"cloudControllerManagerAWS": "registry.ci.openshift.org/openshift:aws-cloud-controller-manager",
"cloudControllerManagerOpenStack": "registry.ci.openshift.org/openshift:openstack-cloud-controller-manager"
Expand All @@ -266,15 +283,29 @@ func TestComposeConfig(t *testing.T) {
},
}, {
name: "Broken JSON is rejected",
infra: &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
ControlPlaneTopology: configv1.SingleReplicaTopologyMode,
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.AWSPlatformType,
},
},
},
imagesContent: `{
"cloudControllerManagerAWS": BAD,
}`,
expectError: "invalid character 'B' looking for beginning of value",
}, {
name: "Single Replica",
namespace: defaultManagementNamespace,
platform: configv1.OpenStackPlatformType,
isSingleReplica: true,
name: "Single Replica",
namespace: defaultManagementNamespace,
infra: &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
ControlPlaneTopology: configv1.SingleReplicaTopologyMode,
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.OpenStackPlatformType,
},
},
},
imagesContent: `{
"cloudControllerManagerAWS": "registry.ci.openshift.org/openshift:aws-cloud-controller-manager",
"cloudControllerManagerOpenStack": "registry.ci.openshift.org/openshift:openstack-cloud-controller-manager"
Expand All @@ -285,6 +316,21 @@ func TestComposeConfig(t *testing.T) {
Platform: configv1.OpenStackPlatformType,
IsSingleReplica: true,
Comment thread
JoelSpeed marked this conversation as resolved.
},
}, {
name: "Empty infrastructure should return error",
expectError: "platform status is not populated on infrastructure",
}, {
name: "Unpopulated infrastructure should return error",
infra: &configv1.Infrastructure{},
expectError: "platform status is not populated on infrastructure",
}, {
name: "Unpopulated infrastructure status should return error",
infra: &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
PlatformStatus: nil,
},
},
expectError: "platform status is not populated on infrastructure",
}}

for _, tc := range tc {
Expand All @@ -297,7 +343,7 @@ func TestComposeConfig(t *testing.T) {
_, err = file.WriteString(tc.imagesContent)
assert.NoError(t, err)

config, err := ComposeConfig(tc.platform, path, tc.namespace, tc.isSingleReplica)
config, err := ComposeConfig(tc.infra, path, tc.namespace)
if tc.expectError != "" {
assert.EqualError(t, err, tc.expectError)
} else {
Expand Down
18 changes: 2 additions & 16 deletions pkg/controllers/clusteroperator_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,20 +97,8 @@ func (r *CloudOperatorReconciler) Reconcile(ctx context.Context, _ ctrl.Request)
return ctrl.Result{}, err
}

platform, err := config.GetProviderFromInfrastructure(infra)
Comment thread
Danil-Grigorev marked this conversation as resolved.
if err != nil {
klog.Errorf("Unable to determine platform from infrastructure: %s", err)
// Ignoring error here as infrastructure resource needs to be reconciled externally
// to provide correct platform
if err := r.setStatusAvailable(ctx); err != nil {
klog.Errorf("Unable to sync cluster operator status: %s", err)
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}

// Verify FeatureGate ExternalCloudProvider is enabled for operator to work in TP phase
external, err := cloudprovider.IsCloudProviderExternal(platform, featureGate)
external, err := cloudprovider.IsCloudProviderExternal(infra.Status.PlatformStatus, featureGate)
Comment thread
JoelSpeed marked this conversation as resolved.
if err != nil {
klog.Errorf("Could not determine external cloud provider state: %v", err)

Expand All @@ -130,9 +118,7 @@ func (r *CloudOperatorReconciler) Reconcile(ctx context.Context, _ ctrl.Request)
return ctrl.Result{}, nil
}

isSingleReplica := infra.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode

config, err := config.ComposeConfig(platform, r.ImagesFile, r.ManagedNamespace, isSingleReplica)
config, err := config.ComposeConfig(infra, r.ImagesFile, r.ManagedNamespace)
if err != nil {
klog.Errorf("Unable to build operator config %s", err)
if err := r.setStatusDegraded(ctx, err); err != nil {
Expand Down
Loading