From 4213d71242858f182f0fa1c1fb0a8fe0a1497f36 Mon Sep 17 00:00:00 2001
From: fabriziopandini <fpandini@vmware.com>
Date: Thu, 9 Jan 2020 19:26:40 +0100
Subject: [PATCH] address comments

---
 cmd/clusterctl/hack/local-overrides.py        | 31 +++++++++----------
 .../pkg/client/config/providers_client.go     |  2 +-
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/cmd/clusterctl/hack/local-overrides.py b/cmd/clusterctl/hack/local-overrides.py
index fb11b2470bba..c49e1fc484d0 100755
--- a/cmd/clusterctl/hack/local-overrides.py
+++ b/cmd/clusterctl/hack/local-overrides.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 
-# Copyright 2015 The Kubernetes Authors.
+# Copyright 2020 The Kubernetes Authors.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -25,7 +25,7 @@
 # - there should be a sigs.k8s.io/cluster-api/clusterctl-settings.json file with the list of provider for which
 #   the local overrides should be generated and the list of provider repositories to be included (on top of cluster-api).
 # {
-#    "providers": [ "cluster-api", "kubeadm", "aws"],
+#    "providers": [ "cluster-api", "kubeadm-bootstrap", "aws"],
 #    "provider_repos": ["../cluster-api-provider-aws"]
 # }
 # - for each additional provider repository there should be a sigs.k8s.io/<provider_repo>/clusterctl-settings.json file e.g.
@@ -52,7 +52,7 @@
               'nextVersion': 'v0.3.0',
               'type': 'CoreProvider',
       },
-      'kubeadm': {
+      'kubeadm-bootstrap': {
             'componentsFile': 'bootstrap-components.yaml',
             'nextVersion': 'v0.3.0',
             'type': 'BootstrapProvider',
@@ -60,6 +60,8 @@
       },
 }
 
+validTypes = ['CoreProvider','BootstrapProvider','InfrastructureProvider']
+
 def load_settings():
     global settings
     try:
@@ -67,7 +69,7 @@ def load_settings():
     except  Exception as e:
         raise Exception('failed to load clusterctl-settings.json: {}'.format(e))
 
-def load_provider():
+def load_providers():
     provider_repos = settings.get('provider_repos', [])
     for repo in provider_repos:
         file = repo + '/clusterctl-settings.json'
@@ -80,7 +82,7 @@ def load_provider():
         except  Exception as e:
             raise Exception('failed to load clusterctl-settings.json from repo {}: {}'.format(repo, e))
 
-def kustomize(args):
+def execCmd(args):
     try:
         out = subprocess.Popen(args,
             stdout=subprocess.PIPE,
@@ -115,32 +117,27 @@ def write_local_override(provider, version, components_file, components_yaml):
 
 def create_local_overrides():
     providerList = settings.get('providers', [])
-    assert providerList is not None, 'invalid configuration: please define the list of providers to overrides'
+    assert providerList is not None, 'invalid configuration: please define the list of providers to override'
     assert len(providerList)>0, 'invalid configuration: please define at least one provider to override'
 
     for provider in providerList:
         p = providers.get(provider)
-        assert p is not None, 'invalid configuration: please the configuration for the {} provider'.format(provider)
-
-        repo = '.'
-        if 'repo' in p:
-            repo = p.get('repo')
+        assert p is not None, 'invalid configuration: please specify the configuration for the {} provider'.format(provider)
 
-        config_folder = '/config/default'
-        if 'configFolder' in p:
-            config_folder = p.get('configFolder')
+        repo = p.get('repo', '.')
+        config_folder = p.get('configFolder', '/config/default')
 
         next_version = p.get('nextVersion')
         assert next_version is not None, 'invalid configuration for provider {}: please provide nextVersion value'.format(provider)
 
         type = p.get('type')
         assert type is not None, 'invalid configuration for provider {}: please provide type value'.format(provider)
-        assert type in ['CoreProvider','BootstrapProvider','InfrastructureProvider'], 'invalid configuration for provider {}: please provide type value'.format(provider)
+        assert type in validTypes, 'invalid configuration for provider {}: please use one of {}'.format(provider, ', '.join(validTypes))
 
         components_file = p.get('componentsFile')
         assert components_file is not None, 'invalid configuration for provider {}: please provide componentsFile value'.format(provider)
 
-        components_yaml = kustomize(['kustomize', 'build', repo + config_folder])
+        components_yaml = execCmd(['kustomize', 'build', repo + config_folder])
         write_local_override(provider, next_version, components_file, components_yaml)
 
         yield provider, type, next_version
@@ -177,7 +174,7 @@ def print_instructions(overrides):
 
 load_settings()
 
-load_provider()
+load_providers()
 
 overrides = create_local_overrides()
 
diff --git a/cmd/clusterctl/pkg/client/config/providers_client.go b/cmd/clusterctl/pkg/client/config/providers_client.go
index 9c91e8c96ccb..3d13faa9b125 100644
--- a/cmd/clusterctl/pkg/client/config/providers_client.go
+++ b/cmd/clusterctl/pkg/client/config/providers_client.go
@@ -29,7 +29,7 @@ import (
 
 const (
 	ClusterAPIName               = "cluster-api"
-	KubeadmBootstrapProviderName = "kubeadm"
+	KubeadmBootstrapProviderName = "kubeadm-bootstrap"
 	ProvidersConfigKey           = "providers"
 )