e2e test for node deleted in cloud provider

This commit is contained in:
andrewsykim
2019-01-14 20:29:10 -05:00
parent 697c2316fa
commit 596c6fbf03
16 changed files with 228 additions and 47 deletions

View File

@@ -1332,7 +1332,7 @@ func extractNodeAddresses(instance *ec2.Instance) ([]v1.NodeAddress, error) {
// This method will not be called from the node that is requesting this ID. i.e. metadata service
// and other local methods cannot be used here
func (c *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, error) {
instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
if err != nil {
return nil, err
}
@@ -1348,7 +1348,7 @@ func (c *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string
// InstanceExistsByProviderID returns true if the instance with the given provider id still exists.
// If false is returned with no error, the instance will be immediately deleted by the cloud controller manager.
func (c *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) {
instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
if err != nil {
return false, err
}
@@ -1379,7 +1379,7 @@ func (c *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID strin
// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes
func (c *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) {
instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
if err != nil {
return false, err
}
@@ -1435,7 +1435,7 @@ func (c *Cloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string
// This method will not be called from the node that is requesting this ID. i.e. metadata service
// and other local methods cannot be used here
func (c *Cloud) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) {
instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
if err != nil {
return "", err
}
@@ -1521,7 +1521,7 @@ func (c *Cloud) GetZone(ctx context.Context) (cloudprovider.Zone, error) {
// This is particularly useful in external cloud providers where the kubelet
// does not initialize node data.
func (c *Cloud) GetZoneByProviderID(ctx context.Context, providerID string) (cloudprovider.Zone, error) {
instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
if err != nil {
return cloudprovider.Zone{}, err
}
@@ -1602,7 +1602,7 @@ func newAWSInstance(ec2Service EC2, instance *ec2.Instance) *awsInstance {
// Gets the full information about this instance from the EC2 API
func (i *awsInstance) describeInstance() (*ec2.Instance, error) {
return describeInstance(i.ec2, awsInstanceID(i.awsID))
return describeInstance(i.ec2, InstanceID(i.awsID))
}
// Gets the mountDevice already assigned to the volume, or assigns an unused mountDevice.
@@ -3787,7 +3787,7 @@ func (c *Cloud) getTaggedSecurityGroups() (map[string]*ec2.SecurityGroup, error)
// Open security group ingress rules on the instances so that the load balancer can talk to them
// Will also remove any security groups ingress rules for the load balancer that are _not_ needed for allInstances
func (c *Cloud) updateInstanceSecurityGroupsForLoadBalancer(lb *elb.LoadBalancerDescription, instances map[awsInstanceID]*ec2.Instance) error {
func (c *Cloud) updateInstanceSecurityGroupsForLoadBalancer(lb *elb.LoadBalancerDescription, instances map[InstanceID]*ec2.Instance) error {
if c.cfg.Global.DisableSecurityGroupIngress {
return nil
}

View File

@@ -863,7 +863,7 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLBTraffic(actualGroups []*ec2.Se
}
// Add SG rules for a given NLB
func (c *Cloud) updateInstanceSecurityGroupsForNLB(mappings []nlbPortMapping, instances map[awsInstanceID]*ec2.Instance, lbName string, clientCidrs []string) error {
func (c *Cloud) updateInstanceSecurityGroupsForNLB(mappings []nlbPortMapping, instances map[InstanceID]*ec2.Instance, lbName string, clientCidrs []string) error {
if c.cfg.Global.DisableSecurityGroupIngress {
return nil
}
@@ -1380,7 +1380,7 @@ func (c *Cloud) ensureLoadBalancerHealthCheck(loadBalancer *elb.LoadBalancerDesc
}
// Makes sure that exactly the specified hosts are registered as instances with the load balancer
func (c *Cloud) ensureLoadBalancerInstances(loadBalancerName string, lbInstances []*elb.Instance, instanceIDs map[awsInstanceID]*ec2.Instance) error {
func (c *Cloud) ensureLoadBalancerInstances(loadBalancerName string, lbInstances []*elb.Instance, instanceIDs map[InstanceID]*ec2.Instance) error {
expected := sets.NewString()
for id := range instanceIDs {
expected.Insert(string(id))
@@ -1557,7 +1557,7 @@ func proxyProtocolEnabled(backend *elb.BackendServerDescription) bool {
// findInstancesForELB gets the EC2 instances corresponding to the Nodes, for setting up an ELB
// We ignore Nodes (with a log message) where the instanceid cannot be determined from the provider,
// and we ignore instances which are not found
func (c *Cloud) findInstancesForELB(nodes []*v1.Node) (map[awsInstanceID]*ec2.Instance, error) {
func (c *Cloud) findInstancesForELB(nodes []*v1.Node) (map[InstanceID]*ec2.Instance, error) {
// Map to instance ids ignoring Nodes where we cannot find the id (but logging)
instanceIDs := mapToAWSInstanceIDsTolerant(nodes)

View File

@@ -34,26 +34,26 @@ import (
// awsInstanceRegMatch represents Regex Match for AWS instance.
var awsInstanceRegMatch = regexp.MustCompile("^i-[^/]*$")
// awsInstanceID represents the ID of the instance in the AWS API, e.g. i-12345678
// InstanceID represents the ID of the instance in the AWS API, e.g. i-12345678
// The "traditional" format is "i-12345678"
// A new longer format is also being introduced: "i-12345678abcdef01"
// We should not assume anything about the length or format, though it seems
// reasonable to assume that instances will continue to start with "i-".
type awsInstanceID string
type InstanceID string
func (i awsInstanceID) awsString() *string {
func (i InstanceID) awsString() *string {
return aws.String(string(i))
}
// kubernetesInstanceID represents the id for an instance in the kubernetes API;
// KubernetesInstanceID represents the id for an instance in the kubernetes API;
// the following form
// * aws:///<zone>/<awsInstanceId>
// * aws:////<awsInstanceId>
// * <awsInstanceId>
type kubernetesInstanceID string
type KubernetesInstanceID string
// mapToAWSInstanceID extracts the awsInstanceID from the kubernetesInstanceID
func (name kubernetesInstanceID) mapToAWSInstanceID() (awsInstanceID, error) {
// MapToAWSInstanceID extracts the InstanceID from the KubernetesInstanceID
func (name KubernetesInstanceID) MapToAWSInstanceID() (InstanceID, error) {
s := string(name)
if !strings.HasPrefix(s, "aws://") {
@@ -85,17 +85,17 @@ func (name kubernetesInstanceID) mapToAWSInstanceID() (awsInstanceID, error) {
return "", fmt.Errorf("Invalid format for AWS instance (%s)", name)
}
return awsInstanceID(awsID), nil
return InstanceID(awsID), nil
}
// mapToAWSInstanceID extracts the awsInstanceIDs from the Nodes, returning an error if a Node cannot be mapped
func mapToAWSInstanceIDs(nodes []*v1.Node) ([]awsInstanceID, error) {
var instanceIDs []awsInstanceID
// mapToAWSInstanceID extracts the InstanceIDs from the Nodes, returning an error if a Node cannot be mapped
func mapToAWSInstanceIDs(nodes []*v1.Node) ([]InstanceID, error) {
var instanceIDs []InstanceID
for _, node := range nodes {
if node.Spec.ProviderID == "" {
return nil, fmt.Errorf("node %q did not have ProviderID set", node.Name)
}
instanceID, err := kubernetesInstanceID(node.Spec.ProviderID).mapToAWSInstanceID()
instanceID, err := KubernetesInstanceID(node.Spec.ProviderID).MapToAWSInstanceID()
if err != nil {
return nil, fmt.Errorf("unable to parse ProviderID %q for node %q", node.Spec.ProviderID, node.Name)
}
@@ -105,15 +105,15 @@ func mapToAWSInstanceIDs(nodes []*v1.Node) ([]awsInstanceID, error) {
return instanceIDs, nil
}
// mapToAWSInstanceIDsTolerant extracts the awsInstanceIDs from the Nodes, skipping Nodes that cannot be mapped
func mapToAWSInstanceIDsTolerant(nodes []*v1.Node) []awsInstanceID {
var instanceIDs []awsInstanceID
// mapToAWSInstanceIDsTolerant extracts the InstanceIDs from the Nodes, skipping Nodes that cannot be mapped
func mapToAWSInstanceIDsTolerant(nodes []*v1.Node) []InstanceID {
var instanceIDs []InstanceID
for _, node := range nodes {
if node.Spec.ProviderID == "" {
klog.Warningf("node %q did not have ProviderID set", node.Name)
continue
}
instanceID, err := kubernetesInstanceID(node.Spec.ProviderID).mapToAWSInstanceID()
instanceID, err := KubernetesInstanceID(node.Spec.ProviderID).MapToAWSInstanceID()
if err != nil {
klog.Warningf("unable to parse ProviderID %q for node %q", node.Spec.ProviderID, node.Name)
continue
@@ -125,7 +125,7 @@ func mapToAWSInstanceIDsTolerant(nodes []*v1.Node) []awsInstanceID {
}
// Gets the full information about this instance from the EC2 API
func describeInstance(ec2Client EC2, instanceID awsInstanceID) (*ec2.Instance, error) {
func describeInstance(ec2Client EC2, instanceID InstanceID) (*ec2.Instance, error) {
request := &ec2.DescribeInstancesInput{
InstanceIds: []*string{instanceID.awsString()},
}
@@ -164,9 +164,9 @@ func (c *instanceCache) describeAllInstancesUncached() (*allInstancesSnapshot, e
return nil, err
}
m := make(map[awsInstanceID]*ec2.Instance)
m := make(map[InstanceID]*ec2.Instance)
for _, i := range instances {
id := awsInstanceID(aws.StringValue(i.InstanceId))
id := InstanceID(aws.StringValue(i.InstanceId))
m[id] = i
}
@@ -191,9 +191,9 @@ type cacheCriteria struct {
// If set to 0 (i.e. unset), cached values will not time out because of age.
MaxAge time.Duration
// HasInstances is a list of awsInstanceIDs that must be in a cached snapshot for it to be considered valid.
// HasInstances is a list of InstanceIDs that must be in a cached snapshot for it to be considered valid.
// If an instance is not found in the cached snapshot, the snapshot be ignored and we will re-fetch.
HasInstances []awsInstanceID
HasInstances []InstanceID
}
// describeAllInstancesCached returns all instances, using cached results if applicable
@@ -257,12 +257,12 @@ func (s *allInstancesSnapshot) MeetsCriteria(criteria cacheCriteria) bool {
// along with the timestamp for cache-invalidation purposes
type allInstancesSnapshot struct {
timestamp time.Time
instances map[awsInstanceID]*ec2.Instance
instances map[InstanceID]*ec2.Instance
}
// FindInstances returns the instances corresponding to the specified ids. If an id is not found, it is ignored.
func (s *allInstancesSnapshot) FindInstances(ids []awsInstanceID) map[awsInstanceID]*ec2.Instance {
m := make(map[awsInstanceID]*ec2.Instance)
func (s *allInstancesSnapshot) FindInstances(ids []InstanceID) map[InstanceID]*ec2.Instance {
m := make(map[InstanceID]*ec2.Instance)
for _, id := range ids {
instance := s.instances[id]
if instance != nil {

View File

@@ -29,8 +29,8 @@ import (
func TestMapToAWSInstanceIDs(t *testing.T) {
tests := []struct {
Kubernetes kubernetesInstanceID
Aws awsInstanceID
Kubernetes KubernetesInstanceID
Aws InstanceID
ExpectError bool
}{
{
@@ -80,7 +80,7 @@ func TestMapToAWSInstanceIDs(t *testing.T) {
}
for _, test := range tests {
awsID, err := test.Kubernetes.mapToAWSInstanceID()
awsID, err := test.Kubernetes.MapToAWSInstanceID()
if err != nil {
if !test.ExpectError {
t.Errorf("unexpected error parsing %s: %v", test.Kubernetes, err)
@@ -139,18 +139,18 @@ func TestSnapshotMeetsCriteria(t *testing.T) {
t.Errorf("Snapshot did not honor MaxAge")
}
if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []awsInstanceID{awsInstanceID("i-12345678")}}) {
if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}) {
t.Errorf("Snapshot did not honor HasInstances with missing instances")
}
snapshot.instances = make(map[awsInstanceID]*ec2.Instance)
snapshot.instances[awsInstanceID("i-12345678")] = &ec2.Instance{}
snapshot.instances = make(map[InstanceID]*ec2.Instance)
snapshot.instances[InstanceID("i-12345678")] = &ec2.Instance{}
if !snapshot.MeetsCriteria(cacheCriteria{HasInstances: []awsInstanceID{awsInstanceID("i-12345678")}}) {
if !snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}) {
t.Errorf("Snapshot did not honor HasInstances with matching instances")
}
if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []awsInstanceID{awsInstanceID("i-12345678"), awsInstanceID("i-00000000")}}) {
if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678"), InstanceID("i-00000000")}}) {
t.Errorf("Snapshot did not honor HasInstances with partially matching instances")
}
}
@@ -170,22 +170,22 @@ func TestOlderThan(t *testing.T) {
func TestSnapshotFindInstances(t *testing.T) {
snapshot := &allInstancesSnapshot{}
snapshot.instances = make(map[awsInstanceID]*ec2.Instance)
snapshot.instances = make(map[InstanceID]*ec2.Instance)
{
id := awsInstanceID("i-12345678")
id := InstanceID("i-12345678")
snapshot.instances[id] = &ec2.Instance{InstanceId: id.awsString()}
}
{
id := awsInstanceID("i-23456789")
id := InstanceID("i-23456789")
snapshot.instances[id] = &ec2.Instance{InstanceId: id.awsString()}
}
instances := snapshot.FindInstances([]awsInstanceID{awsInstanceID("i-12345678"), awsInstanceID("i-23456789"), awsInstanceID("i-00000000")})
instances := snapshot.FindInstances([]InstanceID{InstanceID("i-12345678"), InstanceID("i-23456789"), InstanceID("i-00000000")})
if len(instances) != 2 {
t.Errorf("findInstances returned %d results, expected 2", len(instances))
}
for _, id := range []awsInstanceID{awsInstanceID("i-12345678"), awsInstanceID("i-23456789")} {
for _, id := range []InstanceID{InstanceID("i-12345678"), InstanceID("i-23456789")} {
i := instances[id]
if i == nil {
t.Errorf("findInstances did not return %s", id)