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

AWS: Skip instances that are taggged as a master #41702

Merged
merged 1 commit into from
Mar 1, 2017
Merged
Changes from all commits
Commits
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
27 changes: 20 additions & 7 deletions pkg/cloudprovider/providers/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ const (
createTagSteps = 9
)

// awsTagNameMasterRoles is a set of well-known AWS tag names that indicate the instance is a master
// The major consequence is that it is then not considered for AWS zone discovery for dynamic volume creation.
var awsTagNameMasterRoles = sets.NewString("kubernetes.io/role/master", "k8s.io/role/master")
Copy link
Contributor

Choose a reason for hiding this comment

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

which of them is more standard? kubernetes.io or k8s.io?


// Maps from backend protocol to ELB protocol
var backendProtocolMapping = map[string]string{
"https": "https",
Expand Down Expand Up @@ -1040,9 +1044,9 @@ func (c *Cloud) getInstancesByRegex(regex string) ([]types.NodeName, error) {
return matchingInstances, nil
}

// getAllZones retrieves a list of all the zones in which nodes are running
// getCandidateZonesForDynamicVolume retrieves a list of all the zones in which nodes are running
// It currently involves querying all instances
func (c *Cloud) getAllZones() (sets.String, error) {
func (c *Cloud) getCandidateZonesForDynamicVolume() (sets.String, error) {
// We don't currently cache this; it is currently used only in volume
// creation which is expected to be a comparatively rare occurrence.

Expand All @@ -1066,10 +1070,19 @@ func (c *Cloud) getAllZones() (sets.String, error) {
zones := sets.NewString()

for _, instance := range instances {
// Only return fully-ready instances when listing instances
// (vs a query by name, where we will return it if we find it)
if orEmpty(instance.State.Name) == "pending" {
glog.V(2).Infof("Skipping EC2 instance (pending): %s", *instance.InstanceId)
// We skip over master nodes, if the installation tool labels them with one of the well-known master labels
// This avoids creating a volume in a zone where only the master is running - e.g. #34583
// This is a short-term workaround until the scheduler takes care of zone selection
master := false
for _, tag := range instance.Tags {
tagKey := aws.StringValue(tag.Key)
if awsTagNameMasterRoles.Has(tagKey) {
master = true
}
}

if master {
glog.V(4).Infof("Ignoring master instance %q in zone discovery", aws.StringValue(instance.InstanceId))
continue
}

Expand Down Expand Up @@ -1583,7 +1596,7 @@ func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName)

// CreateDisk implements Volumes.CreateDisk
func (c *Cloud) CreateDisk(volumeOptions *VolumeOptions) (KubernetesVolumeID, error) {
allZones, err := c.getAllZones()
allZones, err := c.getCandidateZonesForDynamicVolume()
if err != nil {
return "", fmt.Errorf("error querying for all zones: %v", err)
}
Expand Down