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.

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

Jump to bottom

Mark cloud provider InstanceV2 as experimental and remove provider ID references #93582

Merged
merged 3 commits into from Aug 5, 2020
Merged

Mark cloud provider InstanceV2 as experimental and remove provider ID references #93582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc1e0d4
cloud provider: remove provider ID references and improve documentation
andrewsykim Jul 30, 2020
82bdba9
cloud provider: remove redundant implementation of InstanceMetadataBy…
andrewsykim Jul 30, 2020
46d522d
cloud-node-lifecycle controller: add missing instancev2 calls for nod…
andrewsykim Aug 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 34 additions & 13 deletions staging/src/k8s.io/cloud-provider/cloud.go
View file
Open in desktop
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
24 changes: 13 additions & 11 deletions staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go
View file
Open in desktop
Expand Up @@ -108,7 +108,9 @@ func NewCloudNodeController(
klog.Infof("Sending events to api server.")
eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")})

if _, ok := cloud.Instances(); !ok {
_, instancesSupported := cloud.Instances()
_, instancesV2Supported := cloud.InstancesV2()
if !instancesSupported && !instancesV2Supported {
return nil, errors.New("cloud provider does not support instances")
}

Expand Down Expand Up @@ -225,7 +227,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.
Expand Down Expand Up @@ -435,9 +437,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.
Expand All @@ -454,12 +456,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
}
Expand All @@ -470,15 +472,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
})
}

Expand Down
27 changes: 18 additions & 9 deletions staging/src/k8s.io/cloud-provider/controllers/nodelifecycle/node_lifecycle_controller.go
View file
Open in desktop
Expand Up @@ -85,7 +85,9 @@ func NewCloudNodeLifecycleController(
return nil, errors.New("no cloud provider provided")
}

if _, ok := cloud.Instances(); !ok {
_, instancesSupported := cloud.Instances()
_, instancesV2Supported := cloud.InstancesV2()
if !instancesSupported && !instancesV2Supported {
return nil, errors.New("cloud provider does not support instances")
}

Expand Down Expand Up @@ -118,12 +120,6 @@ func (c *CloudNodeLifecycleController) Run(stopCh <-chan struct{}) {
// or shutdown. If deleted, it deletes the node resource. If shutdown it
// applies a shutdown taint to the node
func (c *CloudNodeLifecycleController) MonitorNodes() {
instances, ok := c.cloud.Instances()
if !ok {
utilruntime.HandleError(fmt.Errorf("failed to get instances from cloud provider"))
return
}

nodes, err := c.nodeLister.List(labels.Everything())
if err != nil {
klog.Errorf("error listing nodes from cache: %s", err)
Expand All @@ -148,7 +144,7 @@ func (c *CloudNodeLifecycleController) MonitorNodes() {

// At this point the node has NotReady status, we need to check if the node has been removed
// from the cloud provider. If node cannot be found in cloudprovider, then delete the node
exists, err := ensureNodeExistsByProviderID(context.TODO(), instances, node)
exists, err := ensureNodeExistsByProviderID(context.TODO(), c.cloud, node)
if err != nil {
klog.Errorf("error checking if node %s exists: %v", node.Name, err)
continue
Expand Down Expand Up @@ -195,6 +191,10 @@ func (c *CloudNodeLifecycleController) MonitorNodes() {

// shutdownInCloudProvider returns true if the node is shutdown on the cloud provider
func shutdownInCloudProvider(ctx context.Context, cloud cloudprovider.Interface, node *v1.Node) (bool, error) {
if instanceV2, ok := cloud.InstancesV2(); ok {
return instanceV2.InstanceShutdown(ctx, node)
}

instances, ok := cloud.Instances()
if !ok {
return false, errors.New("cloud provider does not support instances")
Expand All @@ -210,7 +210,16 @@ func shutdownInCloudProvider(ctx context.Context, cloud cloudprovider.Interface,

// ensureNodeExistsByProviderID checks if the instance exists by the provider id,
// If provider id in spec is empty it calls instanceId with node name to get provider id
func ensureNodeExistsByProviderID(ctx context.Context, instances cloudprovider.Instances, node *v1.Node) (bool, error) {
func ensureNodeExistsByProviderID(ctx context.Context, cloud cloudprovider.Interface, node *v1.Node) (bool, error) {
if instanceV2, ok := cloud.InstancesV2(); ok {
return instanceV2.InstanceExists(ctx, node)
}

instances, ok := cloud.Instances()
if !ok {
return false, errors.New("instances interface not supported in the cloud provider")
}

providerID := node.Spec.ProviderID
if providerID == "" {
var err error
Expand Down