Merge pull request #52628 from richardalberto/metadata-search-order

Automatic merge from submit-queue (batch tested with PRs 53157, 52628). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Added openstack instance metadata search order

**What this PR does / why we need it**: This PR adds a search order for the instance metadata retrieval on openstack. More information and discussion can be found on #52378 

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #52378 

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

pkg/cloudprovider/providers/openstack/metadata.go

@@ -32,17 +32,25 @@ import (
 	"k8s.io/utils/exec"
 )
 
-// metadataUrl is URL to OpenStack metadata server. It's hardcoded IPv4
+const (
-// link-local address as documented in "OpenStack Cloud Administrator Guide",
+	// metadataUrl is URL to OpenStack metadata server. It's hardcoded IPv4
-// chapter Compute - Networking with nova-network.
+	// link-local address as documented in "OpenStack Cloud Administrator Guide",
-// https://docs.openstack.org/admin-guide/compute-networking-nova.html#metadata-service
+	// chapter Compute - Networking with nova-network.
-const metadataUrl = "http://169.254.169.254/openstack/2012-08-10/meta_data.json"
+	// https://docs.openstack.org/admin-guide/compute-networking-nova.html#metadata-service
+	metadataUrl = "http://169.254.169.254/openstack/2012-08-10/meta_data.json"
 
-// Config drive is defined as an iso9660 or vfat (deprecated) drive
+	// metadataID is used as an identifier on the metadata search order configuration.
-// with the "config-2" label.
+	metadataID = "metadataService"
-// http://docs.openstack.org/user-guide/cli-config-drive.html
+
-const configDriveLabel = "config-2"
+	// Config drive is defined as an iso9660 or vfat (deprecated) drive
-const configDrivePath = "openstack/2012-08-10/meta_data.json"
+	// with the "config-2" label.
+	// http://docs.openstack.org/user-guide/cli-config-drive.html
+	configDriveLabel = "config-2"
+	configDrivePath  = "openstack/2012-08-10/meta_data.json"
+
+	// configDriveID is used as an identifier on the metadata search order configuration.
+	configDriveID = "configDrive"
+)
 
 var ErrBadMetadata = errors.New("Invalid OpenStack metadata, got empty uuid")
 
@@ -141,12 +149,28 @@ func getMetadataFromMetadataService() (*Metadata, error) {
 // Metadata is fixed for the current host, so cache the value process-wide
 var metadataCache *Metadata
 
-func getMetadata() (*Metadata, error) {
+func getMetadata(order string) (*Metadata, error) {
 	if metadataCache == nil {
-		md, err := getMetadataFromConfigDrive()
+		var md *Metadata
-		if err != nil {
+		var err error
-			md, err = getMetadataFromMetadataService()
+
+		elements := strings.Split(order, ",")
+		for _, id := range elements {
+			id = strings.TrimSpace(id)
+			switch id {
+			case configDriveID:
+				md, err = getMetadataFromConfigDrive()
+			case metadataID:
+				md, err = getMetadataFromMetadataService()
+			default:
+				err = fmt.Errorf("%s is not a valid metadata search order option. Supported options are %s and %s", id, configDriveID, metadataID)
+			}
+
+			if err == nil {
+				break
+			}
 		}
+
 		if err != nil {
 			return nil, err
 		}

pkg/cloudprovider/providers/openstack/openstack.go

@@ -100,13 +100,18 @@ type RouterOpts struct {
 	RouterId string `gcfg:"router-id"` // required
 }
 
+type MetadataOpts struct {
+	SearchOrder string `gcfg:"search-order"`
+}
+
 // OpenStack is an implementation of cloud provider Interface for OpenStack.
 type OpenStack struct {
-	provider  *gophercloud.ProviderClient
+	provider     *gophercloud.ProviderClient
-	region    string
+	region       string
-	lbOpts    LoadBalancerOpts
+	lbOpts       LoadBalancerOpts
-	bsOpts    BlockStorageOpts
+	bsOpts       BlockStorageOpts
-	routeOpts RouterOpts
+	routeOpts    RouterOpts
+	metadataOpts MetadataOpts
 	// InstanceID of the server where this OpenStack object is instantiated.
 	localInstanceID string
 }
@@ -128,6 +133,7 @@ type Config struct {
 	LoadBalancer LoadBalancerOpts
 	BlockStorage BlockStorageOpts
 	Route        RouterOpts
+	Metadata     MetadataOpts
 }
 
 func init() {
@@ -181,6 +187,7 @@ func readConfig(config io.Reader) (Config, error) {
 	// Set default values for config params
 	cfg.BlockStorage.BSVersion = "auto"
 	cfg.BlockStorage.TrustDevicePath = false
+	cfg.Metadata.SearchOrder = fmt.Sprintf("%s,%s", configDriveID, metadataID)
 
 	err := gcfg.ReadInto(&cfg, config)
 	return cfg, err
@@ -198,7 +205,7 @@ func (c *Caller) Call(f func()) {
 	}
 }
 
-func readInstanceID() (string, error) {
+func readInstanceID(searchOrder string) (string, error) {
 	// Try to find instance ID on the local filesystem (created by cloud-init)
 	const instanceIDFile = "/var/lib/cloud/data/instance-id"
 	idBytes, err := ioutil.ReadFile(instanceIDFile)
@@ -212,7 +219,7 @@ func readInstanceID() (string, error) {
 		// Fall through to metadata server lookup
 	}
 
-	md, err := getMetadata()
+	md, err := getMetadata(searchOrder)
 	if err != nil {
 		return "", err
 	}
@@ -246,6 +253,10 @@ func checkOpenStackOpts(openstackOpts *OpenStack) error {
 		}
 	}
 
+	if err := checkMetadataSearchOrder(openstackOpts.metadataOpts.SearchOrder); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -280,11 +291,12 @@ func newOpenStack(cfg Config) (*OpenStack, error) {
 	}
 
 	os := OpenStack{
-		provider:  provider,
+		provider:     provider,
-		region:    cfg.Global.Region,
+		region:       cfg.Global.Region,
-		lbOpts:    cfg.LoadBalancer,
+		lbOpts:       cfg.LoadBalancer,
-		bsOpts:    cfg.BlockStorage,
+		bsOpts:       cfg.BlockStorage,
-		routeOpts: cfg.Route,
+		routeOpts:    cfg.Route,
+		metadataOpts: cfg.Metadata,
 	}
 
 	err = checkOpenStackOpts(&os)
@@ -541,7 +553,7 @@ func (os *OpenStack) Zones() (cloudprovider.Zones, bool) {
 }
 
 func (os *OpenStack) GetZone() (cloudprovider.Zone, error) {
-	md, err := getMetadata()
+	md, err := getMetadata(os.metadataOpts.SearchOrder)
 	if err != nil {
 		return cloudprovider.Zone{}, err
 	}
@@ -744,3 +756,28 @@ func (os *OpenStack) volumeService(forceVersion string) (volumeService, error) {
 		return nil, errors.New(err_txt)
 	}
 }
+
+func checkMetadataSearchOrder(order string) error {
+	if order == "" {
+		return errors.New("Invalid value in section [Metadata] with key `search-order`. Value cannot be empty")
+	}
+
+	elements := strings.Split(order, ",")
+	if len(elements) > 2 {
+		return errors.New("Invalid value in section [Metadata] with key `search-order`. Value cannot contain more than 2 elements")
+	}
+
+	for _, id := range elements {
+		id = strings.TrimSpace(id)
+		switch id {
+		case configDriveID:
+		case metadataID:
+		default:
+			errTxt := "Invalid element '%s' found in section [Metadata] with key `search-order`." +
+				"Supported elements include '%s' and '%s'"
+			return fmt.Errorf(errTxt, id, configDriveID, metadataID)
+		}
+	}
+
+	return nil
+}

pkg/cloudprovider/providers/openstack/openstack_instances.go

@@ -32,6 +32,7 @@ import (
 
 type Instances struct {
 	compute *gophercloud.ServiceClient
+	opts    MetadataOpts
 }
 
 // Instances returns an implementation of Instances for OpenStack.
@@ -45,13 +46,16 @@ func (os *OpenStack) Instances() (cloudprovider.Instances, bool) {
 
 	glog.V(1).Info("Claiming to support Instances")
 
-	return &Instances{compute}, true
+	return &Instances{
+		compute: compute,
+		opts:    os.metadataOpts,
+	}, true
 }
 
 // Implementation of Instances.CurrentNodeName
 // Note this is *not* necessarily the same as hostname.
 func (i *Instances) CurrentNodeName(hostname string) (types.NodeName, error) {
-	md, err := getMetadata()
+	md, err := getMetadata(i.opts.SearchOrder)
 	if err != nil {
 		return "", err
 	}
@@ -119,7 +123,7 @@ func (i *Instances) InstanceExistsByProviderID(providerID string) (bool, error)
 // InstanceID returns the kubelet's cloud provider ID.
 func (os *OpenStack) InstanceID() (string, error) {
 	if len(os.localInstanceID) == 0 {
-		id, err := readInstanceID()
+		id, err := readInstanceID(os.metadataOpts.SearchOrder)
 		if err != nil {
 			return "", err
 		}

pkg/cloudprovider/providers/openstack/openstack_test.go

@@ -101,7 +101,8 @@ func TestReadConfig(t *testing.T) {
  [BlockStorage]
  bs-version = auto
  trust-device-path = yes
-
+ [Metadata]
+ search-order = configDrive, metadataService
  `))
 	if err != nil {
 		t.Fatalf("Should succeed when a valid config is provided: %s", err)
@@ -128,6 +129,9 @@ func TestReadConfig(t *testing.T) {
 	if cfg.BlockStorage.BSVersion != "auto" {
 		t.Errorf("incorrect bs.bs-version: %v", cfg.BlockStorage.BSVersion)
 	}
+	if cfg.Metadata.SearchOrder != "configDrive, metadataService" {
+		t.Errorf("incorrect md.search-order: %v", cfg.Metadata.SearchOrder)
+	}
 }
 
 func TestToAuthOptions(t *testing.T) {
@@ -169,6 +173,9 @@ func TestCheckOpenStackOpts(t *testing.T) {
 					ManageSecurityGroups: true,
 					NodeSecurityGroupID:  "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144",
 				},
+				metadataOpts: MetadataOpts{
+					SearchOrder: configDriveID,
+				},
 			},
 			expectedError: nil,
 		},
@@ -187,6 +194,9 @@ func TestCheckOpenStackOpts(t *testing.T) {
 					ManageSecurityGroups: true,
 					NodeSecurityGroupID:  "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144",
 				},
+				metadataOpts: MetadataOpts{
+					SearchOrder: configDriveID,
+				},
 			},
 			expectedError: nil,
 		},
@@ -203,6 +213,9 @@ func TestCheckOpenStackOpts(t *testing.T) {
 					ManageSecurityGroups: true,
 					NodeSecurityGroupID:  "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144",
 				},
+				metadataOpts: MetadataOpts{
+					SearchOrder: configDriveID,
+				},
 			},
 			expectedError: fmt.Errorf("monitor-delay not set in cloud provider config"),
 		},
@@ -221,9 +234,43 @@ func TestCheckOpenStackOpts(t *testing.T) {
 					MonitorMaxRetries:    uint(3),
 					ManageSecurityGroups: true,
 				},
+				metadataOpts: MetadataOpts{
+					SearchOrder: configDriveID,
+				},
 			},
 			expectedError: fmt.Errorf("node-security-group not set in cloud provider config"),
 		},
+		{
+			name: "test5",
+			openstackOpts: &OpenStack{
+				provider: nil,
+				metadataOpts: MetadataOpts{
+					SearchOrder: "",
+				},
+			},
+			expectedError: fmt.Errorf("Invalid value in section [Metadata] with key `search-order`. Value cannot be empty"),
+		},
+		{
+			name: "test6",
+			openstackOpts: &OpenStack{
+				provider: nil,
+				metadataOpts: MetadataOpts{
+					SearchOrder: "value1,value2,value3",
+				},
+			},
+			expectedError: fmt.Errorf("Invalid value in section [Metadata] with key `search-order`. Value cannot contain more than 2 elements"),
+		},
+		{
+			name: "test7",
+			openstackOpts: &OpenStack{
+				provider: nil,
+				metadataOpts: MetadataOpts{
+					SearchOrder: "value1",
+				},
+			},
+			expectedError: fmt.Errorf("Invalid element '%s' found in section [Metadata] with key `search-order`."+
+				"Supported elements include '%s' and '%s'", "value1", configDriveID, metadataID),
+		},
 	}
 
 	for _, testcase := range tests {
@@ -374,6 +421,8 @@ func configFromEnv() (cfg Config, ok bool) {
 		(cfg.Global.TenantId != "" || cfg.Global.TenantName != "" ||
 			cfg.Global.DomainId != "" || cfg.Global.DomainName != ""))
 
+	cfg.Metadata.SearchOrder = fmt.Sprintf("%s,%s", configDriveID, metadataID)
+
 	return
 }