Merge pull request #123224 from mmerkes/cleanup
Clean up dead code in node controller
This commit is contained in:
		@@ -688,29 +688,6 @@ func excludeCloudTaint(taints []v1.Taint) []v1.Taint {
 | 
				
			|||||||
	return newTaints
 | 
						return newTaints
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
// 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) {
 | 
					 | 
				
			||||||
	providerID := node.Spec.ProviderID
 | 
					 | 
				
			||||||
	if providerID == "" {
 | 
					 | 
				
			||||||
		var err error
 | 
					 | 
				
			||||||
		providerID, err = instances.InstanceID(ctx, types.NodeName(node.Name))
 | 
					 | 
				
			||||||
		if err != nil {
 | 
					 | 
				
			||||||
			if err == cloudprovider.InstanceNotFound {
 | 
					 | 
				
			||||||
				return false, nil
 | 
					 | 
				
			||||||
			}
 | 
					 | 
				
			||||||
			return false, err
 | 
					 | 
				
			||||||
		}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
		if providerID == "" {
 | 
					 | 
				
			||||||
			klog.Warningf("Cannot find valid providerID for node name %q, assuming non existence", node.Name)
 | 
					 | 
				
			||||||
			return false, nil
 | 
					 | 
				
			||||||
		}
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	return instances.InstanceExistsByProviderID(ctx, providerID)
 | 
					 | 
				
			||||||
}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
func getNodeAddressesByProviderIDOrName(ctx context.Context, instances cloudprovider.Instances, providerID, nodeName string) ([]v1.NodeAddress, error) {
 | 
					func getNodeAddressesByProviderIDOrName(ctx context.Context, instances cloudprovider.Instances, providerID, nodeName string) ([]v1.NodeAddress, error) {
 | 
				
			||||||
	nodeAddresses, err := instances.NodeAddressesByProviderID(ctx, providerID)
 | 
						nodeAddresses, err := instances.NodeAddressesByProviderID(ctx, providerID)
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -44,128 +44,6 @@ import (
 | 
				
			|||||||
	"github.com/stretchr/testify/assert"
 | 
						"github.com/stretchr/testify/assert"
 | 
				
			||||||
)
 | 
					)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func TestEnsureNodeExistsByProviderID(t *testing.T) {
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	testCases := []struct {
 | 
					 | 
				
			||||||
		testName           string
 | 
					 | 
				
			||||||
		node               *v1.Node
 | 
					 | 
				
			||||||
		expectedCalls      []string
 | 
					 | 
				
			||||||
		expectedNodeExists bool
 | 
					 | 
				
			||||||
		hasInstanceID      bool
 | 
					 | 
				
			||||||
		existsByProviderID bool
 | 
					 | 
				
			||||||
		nodeNameErr        error
 | 
					 | 
				
			||||||
		providerIDErr      error
 | 
					 | 
				
			||||||
	}{
 | 
					 | 
				
			||||||
		{
 | 
					 | 
				
			||||||
			testName:           "node exists by provider id",
 | 
					 | 
				
			||||||
			existsByProviderID: true,
 | 
					 | 
				
			||||||
			providerIDErr:      nil,
 | 
					 | 
				
			||||||
			hasInstanceID:      true,
 | 
					 | 
				
			||||||
			nodeNameErr:        errors.New("unimplemented"),
 | 
					 | 
				
			||||||
			expectedCalls:      []string{"instance-exists-by-provider-id"},
 | 
					 | 
				
			||||||
			expectedNodeExists: true,
 | 
					 | 
				
			||||||
			node: &v1.Node{
 | 
					 | 
				
			||||||
				ObjectMeta: metav1.ObjectMeta{
 | 
					 | 
				
			||||||
					Name: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
				Spec: v1.NodeSpec{
 | 
					 | 
				
			||||||
					ProviderID: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
			},
 | 
					 | 
				
			||||||
		},
 | 
					 | 
				
			||||||
		{
 | 
					 | 
				
			||||||
			testName:           "does not exist by provider id",
 | 
					 | 
				
			||||||
			existsByProviderID: false,
 | 
					 | 
				
			||||||
			providerIDErr:      nil,
 | 
					 | 
				
			||||||
			hasInstanceID:      true,
 | 
					 | 
				
			||||||
			nodeNameErr:        errors.New("unimplemented"),
 | 
					 | 
				
			||||||
			expectedCalls:      []string{"instance-exists-by-provider-id"},
 | 
					 | 
				
			||||||
			expectedNodeExists: false,
 | 
					 | 
				
			||||||
			node: &v1.Node{
 | 
					 | 
				
			||||||
				ObjectMeta: metav1.ObjectMeta{
 | 
					 | 
				
			||||||
					Name: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
				Spec: v1.NodeSpec{
 | 
					 | 
				
			||||||
					ProviderID: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
			},
 | 
					 | 
				
			||||||
		},
 | 
					 | 
				
			||||||
		{
 | 
					 | 
				
			||||||
			testName:           "exists by instance id",
 | 
					 | 
				
			||||||
			existsByProviderID: true,
 | 
					 | 
				
			||||||
			providerIDErr:      nil,
 | 
					 | 
				
			||||||
			hasInstanceID:      true,
 | 
					 | 
				
			||||||
			nodeNameErr:        nil,
 | 
					 | 
				
			||||||
			expectedCalls:      []string{"instance-id", "instance-exists-by-provider-id"},
 | 
					 | 
				
			||||||
			expectedNodeExists: true,
 | 
					 | 
				
			||||||
			node: &v1.Node{
 | 
					 | 
				
			||||||
				ObjectMeta: metav1.ObjectMeta{
 | 
					 | 
				
			||||||
					Name: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
			},
 | 
					 | 
				
			||||||
		},
 | 
					 | 
				
			||||||
		{
 | 
					 | 
				
			||||||
			testName:           "does not exist by no instance id",
 | 
					 | 
				
			||||||
			existsByProviderID: true,
 | 
					 | 
				
			||||||
			providerIDErr:      nil,
 | 
					 | 
				
			||||||
			hasInstanceID:      false,
 | 
					 | 
				
			||||||
			nodeNameErr:        cloudprovider.InstanceNotFound,
 | 
					 | 
				
			||||||
			expectedCalls:      []string{"instance-id"},
 | 
					 | 
				
			||||||
			expectedNodeExists: false,
 | 
					 | 
				
			||||||
			node: &v1.Node{
 | 
					 | 
				
			||||||
				ObjectMeta: metav1.ObjectMeta{
 | 
					 | 
				
			||||||
					Name: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
			},
 | 
					 | 
				
			||||||
		},
 | 
					 | 
				
			||||||
		{
 | 
					 | 
				
			||||||
			testName:           "provider id returns error",
 | 
					 | 
				
			||||||
			existsByProviderID: false,
 | 
					 | 
				
			||||||
			providerIDErr:      errors.New("unimplemented"),
 | 
					 | 
				
			||||||
			hasInstanceID:      true,
 | 
					 | 
				
			||||||
			nodeNameErr:        cloudprovider.InstanceNotFound,
 | 
					 | 
				
			||||||
			expectedCalls:      []string{"instance-exists-by-provider-id"},
 | 
					 | 
				
			||||||
			expectedNodeExists: false,
 | 
					 | 
				
			||||||
			node: &v1.Node{
 | 
					 | 
				
			||||||
				ObjectMeta: metav1.ObjectMeta{
 | 
					 | 
				
			||||||
					Name: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
				Spec: v1.NodeSpec{
 | 
					 | 
				
			||||||
					ProviderID: "node0",
 | 
					 | 
				
			||||||
				},
 | 
					 | 
				
			||||||
			},
 | 
					 | 
				
			||||||
		},
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	for _, tc := range testCases {
 | 
					 | 
				
			||||||
		t.Run(tc.testName, func(t *testing.T) {
 | 
					 | 
				
			||||||
			fc := &fakecloud.Cloud{
 | 
					 | 
				
			||||||
				ExistsByProviderID: tc.existsByProviderID,
 | 
					 | 
				
			||||||
				Err:                tc.nodeNameErr,
 | 
					 | 
				
			||||||
				ErrByProviderID:    tc.providerIDErr,
 | 
					 | 
				
			||||||
			}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
			if tc.hasInstanceID {
 | 
					 | 
				
			||||||
				fc.ExtID = map[types.NodeName]string{
 | 
					 | 
				
			||||||
					types.NodeName(tc.node.Name): "provider-id://a",
 | 
					 | 
				
			||||||
				}
 | 
					 | 
				
			||||||
			}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
			instances, _ := fc.Instances()
 | 
					 | 
				
			||||||
			exists, err := ensureNodeExistsByProviderID(context.TODO(), instances, tc.node)
 | 
					 | 
				
			||||||
			assert.Equal(t, err, tc.providerIDErr)
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
			assert.EqualValues(t, tc.expectedCalls, fc.Calls,
 | 
					 | 
				
			||||||
				"expected cloud provider methods `%v` to be called but `%v` was called ",
 | 
					 | 
				
			||||||
				tc.expectedCalls, fc.Calls)
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
			assert.Equal(t, tc.expectedNodeExists, exists,
 | 
					 | 
				
			||||||
				"expected exists to be `%t` but got `%t`",
 | 
					 | 
				
			||||||
				tc.existsByProviderID, exists)
 | 
					 | 
				
			||||||
		})
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
func Test_syncNode(t *testing.T) {
 | 
					func Test_syncNode(t *testing.T) {
 | 
				
			||||||
	tests := []struct {
 | 
						tests := []struct {
 | 
				
			||||||
		name         string
 | 
							name         string
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user