Skip to content
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.

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

Sync cloudprovider configmap #206

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,118 @@ package cloudprovider
import (
"fmt"

"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/library-go/pkg/operator/configobserver"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/resourcesynccontroller"

"github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/configobservation"
)

// ObserveCloudProviderNames observes the loud provider from the global cluster infrastructure resource.
const (
targetNamespaceName = "openshift-kube-controller-manager"
cloudProviderConfFilePath = "/etc/kubernetes/static-pod-resources/configmaps/cloud-config/%s"
configNamespace = "openshift-config"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derekwaynecarr I think we want this to be openshift-config-managed to be consistent for all clouds and allow future filtering, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, if this has to change https://github.com/openshift/installer/pull/1516/files#diff-8db489d59cedb0a4f04c7546779f58a2R68 should be updated to reflect new location right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

)

// ObserveCloudProviderNames observes the cloud provider from the global cluster infrastructure resource.
func ObserveCloudProviderNames(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
listers := genericListers.(configobservation.Listers)
var errs []error
cloudProvidersPath := []string{"extendedArguments", "cloud-provider"}

cloudProviderConfPath := []string{"extendedArguments", "cloud-config"}
previouslyObservedConfig := map[string]interface{}{}

existingCloudConfig, _, err := unstructured.NestedStringSlice(existingConfig, cloudProviderConfPath...)
if err != nil {
return previouslyObservedConfig, append(errs, err)
}

if currentCloudProvider, _, _ := unstructured.NestedStringSlice(existingConfig, cloudProvidersPath...); len(currentCloudProvider) > 0 {
if err := unstructured.SetNestedStringSlice(previouslyObservedConfig, currentCloudProvider, cloudProvidersPath...); err != nil {
errs = append(errs, err)
}
}

if len(existingCloudConfig) > 0 {
if err := unstructured.SetNestedStringSlice(previouslyObservedConfig, existingCloudConfig, cloudProviderConfPath...); err != nil {
errs = append(errs, err)
}
}

observedConfig := map[string]interface{}{}

infrastructure, err := listers.InfrastructureLister.Get("cluster")
if errors.IsNotFound(err) {
recorder.Warningf("ObserveRestrictedCIDRFailed", "Required infrastructures.%s/cluster not found", configv1.GroupName)
recorder.Warningf("ObserveCloudProviderNames", "Required infrastructures.%s/cluster not found", configv1.GroupName)
return observedConfig, errs
}
if err != nil {
return previouslyObservedConfig, errs
}

cloudProvider := getPlatformName(infrastructure.Status.Platform, recorder)
if len(cloudProvider) > 0 {
if err := unstructured.SetNestedStringSlice(observedConfig, []string{cloudProvider}, cloudProvidersPath...); err != nil {
errs = append(errs, err)
}
}

sourceCloudConfigMap := infrastructure.Spec.CloudConfig.Name
sourceCloudConfigNamespace := configNamespace
sourceLocation := resourcesynccontroller.ResourceLocation{
Namespace: sourceCloudConfigNamespace,
Name: sourceCloudConfigMap,
}
// we set cloudprovider configmap values only for vsphere.
if cloudProvider != "vsphere" {
sourceCloudConfigMap = ""
}

if len(sourceCloudConfigMap) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, I think we need to sync an empty location to clear the destination. Can you have a look at the godoc on syncconfigmap?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

sourceLocation = resourcesynccontroller.ResourceLocation{}
}

err = listers.ResourceSyncer().SyncConfigMap(
resourcesynccontroller.ResourceLocation{
Namespace: targetNamespaceName,
Name: "cloud-config",
},
sourceLocation)

if err != nil {
errs = append(errs, err)
return observedConfig, errs
}

if len(sourceCloudConfigMap) == 0 {
return observedConfig, errs
}

// usually key will be simply config but we should refer it just in case
staticCloudConfFile := fmt.Sprintf(cloudProviderConfFilePath, infrastructure.Spec.CloudConfig.Key)

if err := unstructured.SetNestedStringSlice(observedConfig, []string{staticCloudConfFile}, cloudProviderConfPath...); err != nil {
recorder.Warningf("ObserveCloudProviderNames", "Failed setting cloud-config : %v", err)
errs = append(errs, err)
}

if !equality.Semantic.DeepEqual(existingCloudConfig, []string{staticCloudConfFile}) {
recorder.Eventf("ObserveCloudProviderNamesChanges", "CloudProvider config file changed to %s", staticCloudConfFile)
}

return observedConfig, errs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just before returning, can you look for a diff between the prevconfig and the newconfig and emit an event. @mfojtik probably has an example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ does this work or did you mean deep comparsion of actual cloudprovider configuration?

}

func getPlatformName(platformType configv1.PlatformType, recorder events.Recorder) string {
cloudProvider := ""
switch infrastructure.Status.Platform {
switch platformType {
case "":
recorder.Warningf("ObserveCloudProvidersFailed", "Required status.platform field is not set in infrastructures.%s/cluster", configv1.GroupName)
return previouslyObservedConfig, errs
case configv1.AWSPlatformType:
cloudProvider = "aws"
case configv1.AzurePlatformType:
Expand All @@ -58,12 +131,5 @@ func ObserveCloudProviderNames(genericListers configobserver.Listers, recorder e
// TODO find a way to indicate to the user that we didn't honor their choice
recorder.Warningf("ObserveCloudProvidersFailed", fmt.Sprintf("No recognized cloud provider platform found in infrastructures.%s/cluster.status.platform", configv1.GroupName))
}

if len(cloudProvider) > 0 {
if err := unstructured.SetNestedStringSlice(observedConfig, []string{cloudProvider}, cloudProvidersPath...); err != nil {
errs = append(errs, err)
}
}

return observedConfig, errs
return cloudProvider
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,21 @@ import (
configv1 "github.com/openshift/api/config/v1"
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/resourcesynccontroller"

"github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/configobservation"
)

type FakeResourceSyncer struct{}

func (fakeSyncer *FakeResourceSyncer) SyncConfigMap(destination, source resourcesynccontroller.ResourceLocation) error {
return nil
}

func (fakeSyncer *FakeResourceSyncer) SyncSecret(destination, source resourcesynccontroller.ResourceLocation) error {
return nil
}

func TestObserveCloudProviderNames(t *testing.T) {
cases := []struct {
platform configv1.PlatformType
Expand Down Expand Up @@ -52,6 +63,7 @@ func TestObserveCloudProviderNames(t *testing.T) {
}
listers := configobservation.Listers{
InfrastructureLister: configlistersv1.NewInfrastructureLister(indexer),
ResourceSync: &FakeResourceSyncer{},
}
result, errs := ObserveCloudProviderNames(listers, events.NewInMemoryRecorder("cloud"), map[string]interface{}{})
if len(errs) > 0 {
Expand Down
1 change: 1 addition & 0 deletions pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ var deploymentConfigMaps = []revision.RevisionResource{

{Name: "config"},
{Name: "controller-manager-kubeconfig"},
{Name: "cloud-config", Optional: true},
{Name: "serviceaccount-ca"},
{Name: "service-ca"},
}
Expand Down