cloud provider: remove provider ID references and improve documentation

Signed-off-by: Andrew Sy Kim <[email protected]>

Author: andrewsykim

staging/src/k8s.io/cloud-provider/cloud.go

@@ -49,8 +49,11 @@ type Interface interface {
 	LoadBalancer() (LoadBalancer, bool)
 	// Instances returns an instances interface. Also returns true if the interface is supported, false otherwise.
 	Instances() (Instances, bool)
-	// InstancesV2 is an implementation for instances only used by cloud node-controller now.
+	// InstancesV2 is an implementation for instances and should only be implemented by external cloud providers.
+	// Implementing InstancesV2 is behaviorally identical to Instances but is optimized to significantly reduce
+	// API calls to the cloud provider when registering and syncing nodes.
 	// Also returns true if the interface is supported, false otherwise.
+	// WARNING: InstancesV2 is an experimental interface and is subject to change in v1.20.
 	InstancesV2() (InstancesV2, bool)
 	// Zones returns a zones interface. Also returns true if the interface is supported, false otherwise.
 	Zones() (Zones, bool)
@@ -189,15 +192,20 @@ type Instances interface {
 	InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error)
 }
 
-// InstancesV2 is an abstract, pluggable interface for sets of instances.
-// Unlike Instances, it is only used by cloud node-controller now.
+// InstancesV2 is an abstract, pluggable interface for cloud provider instances.
+// Unlike the Instances interface, it is designed for external cloud providers and should only be used by them.
+// WARNING: InstancesV2 is an experimental interface and is subject to change in v1.20.
 type InstancesV2 interface {
-	// InstanceExistsByProviderID returns true if the instance for the given provider exists.
-	InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error)
-	// InstanceShutdownByProviderID returns true if the instance is shutdown in cloudprovider.
-	InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error)
-	// InstanceMetadataByProviderID returns the instance's metadata.
-	InstanceMetadataByProviderID(ctx context.Context, providerID string) (*InstanceMetadata, error)
+	// InstanceExists returns true if the instance for the given node exists according to the cloud provider.
+	// Use the node.name or node.spec.providerID field to find the node in the cloud provider.
+	InstanceExists(ctx context.Context, node *v1.Node) (bool, error)
+	// InstanceShutdown returns true if the instance is shutdown according to the cloud provider.
+	// Use the node.name or node.spec.providerID field to find the node in the cloud provider.
+	InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error)
+	// InstanceMetadata returns the instance's metadata. The values returned in InstanceMetadata are
+	// translated into specific fields in the Node object on registration.
+	// Use the node.name or node.spec.providerID field to find the node in the cloud provider.
+	InstanceMetadata(ctx context.Context, node *v1.Node) (*InstanceMetadata, error)
 }
 
 // Route is a representation of an advanced routing rule.
@@ -265,12 +273,25 @@ type PVLabeler interface {
 	GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error)
 }
 
-// InstanceMetadata contains metadata about the specific instance.
+// InstanceMetadata contains metadata about a specific instance.
+// Values returned in InstanceMetadata are translated into specific fields in Node.
 type InstanceMetadata struct {
-	// ProviderID is provider's id that instance belongs to.
+	// ProviderID is a unique ID used to idenfitify an instance on the cloud provider.
+	// The ProviderID set here will be set on the node's spec.providerID field.
+	// The provider ID format can be set by the cloud provider but providers should
+	// ensure the format does not change in any incompatible way.
+	//
+	// The provider ID format used by existing cloud provider has been:
+	//    <provider-name>://<instance-id>
+	// Existing providers setting this field should preserve the existing format
+	// currently being set in node.spec.providerID.
 	ProviderID string
-	// Type is instance's type.
-	Type string
+	// InstanceType is the instance's type.
+	// The InstanceType set here will be set using the following labels on the node object:
+	//   * node.kubernetes.io/instance-type=<instance-type>
+	//   * beta.kubernetes.io/instance-type=<instance-type> (DEPRECATED)
+	InstanceType string
 	// NodeAddress contains information for the instance's address.
+	// The node addresses returned here will be set on the node's status.addresses field.
 	NodeAddresses []v1.NodeAddress
 }

staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go

@@ -225,7 +225,7 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.
 
 	instanceMetadataGetter := func(providerID string, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
 		if instancesV2, ok := cnc.cloud.InstancesV2(); instancesV2 != nil && ok {
-			return instancesV2.InstanceMetadataByProviderID(ctx, providerID)
+			return instancesV2.InstanceMetadata(ctx, node)
 		}
 
 		// If InstancesV2 not implement, use Instances.
@@ -435,9 +435,9 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co
 		providerID = node.Spec.ProviderID
 	}
 
-	instanceMetadataGetter := func(providerID string, nodeName string) (*cloudprovider.InstanceMetadata, error) {
+	instanceMetadataGetter := func(providerID string, nodeName string, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
 		if instancesV2, ok := cnc.cloud.InstancesV2(); instancesV2 != nil && ok {
-			return instancesV2.InstanceMetadataByProviderID(ctx, providerID)
+			return instancesV2.InstanceMetadata(ctx, node)
 		}
 
 		// If InstancesV2 not implement, use Instances.
@@ -454,12 +454,12 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co
 			return nil, err
 		}
 		return &cloudprovider.InstanceMetadata{
-			Type:          instanceType,
+			InstanceType:  instanceType,
 			NodeAddresses: nodeAddresses,
 		}, nil
 	}
 
-	instanceMeta, err := instanceMetadataGetter(providerID, node.Name)
+	instanceMeta, err := instanceMetadataGetter(providerID, node.Name, node)
 	if err != nil {
 		return nil, err
 	}
@@ -470,15 +470,15 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co
 		return nil, errors.New("failed to find kubelet node IP from cloud provider")
 	}
 
-	if instanceMeta.Type != "" {
-		klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceType, instanceMeta.Type)
-		klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceTypeStable, instanceMeta.Type)
+	if instanceMeta.InstanceType != "" {
+		klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceType, instanceMeta.InstanceType)
+		klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceTypeStable, instanceMeta.InstanceType)
 		nodeModifiers = append(nodeModifiers, func(n *v1.Node) {
 			if n.Labels == nil {
 				n.Labels = map[string]string{}
 			}
-			n.Labels[v1.LabelInstanceType] = instanceMeta.Type
-			n.Labels[v1.LabelInstanceTypeStable] = instanceMeta.Type
+			n.Labels[v1.LabelInstanceType] = instanceMeta.InstanceType
+			n.Labels[v1.LabelInstanceTypeStable] = instanceMeta.InstanceType
 		})
 	}
 

staging/src/k8s.io/cloud-provider/fake/fake.go

@@ -303,14 +303,27 @@ func (f *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID str
 	return f.NodeShutdown, f.ErrShutdownByProviderID
 }
 
-// InstanceMetadataByProviderID returns metadata of the specified instance.
-func (f *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID string) (*cloudprovider.InstanceMetadata, error) {
+// InstanceExists returns true if the instance corresponding to a node still exists and is running.
+// If false is returned with no error, the instance will be immediately deleted by the cloud controller manager.
+func (f *Cloud) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) {
+	f.addCall("instance-exists")
+	return f.ExistsByProviderID, f.ErrByProviderID
+}
+
+// InstanceShutdown returns true if the instances is in safe state to detach volumes
+func (f *Cloud) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) {
+	f.addCall("instance-shutdown")
+	return f.NodeShutdown, f.ErrShutdownByProviderID
+}
+
+// InstanceMetadata returns metadata of the specified instance.
+func (f *Cloud) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
 	f.addCall("instance-metadata-by-provider-id")
 	f.addressesMux.Lock()
 	defer f.addressesMux.Unlock()
 	return &cloudprovider.InstanceMetadata{
-		ProviderID:    providerID,
-		Type:          f.InstanceTypes[types.NodeName(providerID)],
+		ProviderID:    node.Spec.ProviderID,
+		InstanceType:  f.InstanceTypes[types.NodeName(node.Spec.ProviderID)],
 		NodeAddresses: f.Addresses,
 	}, f.Err
 }