adding details for --resources

* get user input for resources

* Add better testing for failures to add flags

* fix a small issue with --resources

* finalize the --resources

docs/tagging_controller.md

@@ -0,0 +1,7 @@
+# The Tagging Controller
+
+The tagging controller is responsible for tagging and untagging node resources when it joins and leaves the cluster respectively. It can add and remove tags based on user input. Unlike the existing controllers, the tagging controller is working exclusively with AWS as we want to tag the resources (EC instances for example). For functionalities used by the controller, we primarily use `CreateTags` and `DeleteTags` from `EC2`.
+
+| Flag | Valid Values | Default | Description |
+|------| --- | --- | --- |
+| tags | Comma-separated list of key=value | - | A comma-separated list of key-value pairs which will be recorded as nodes' additional tags. For example: "Key1=Val1,Key2=Val2,KeyNoVal1=,KeyNoVal2" |

pkg/controllers/options/resources.go

@@ -0,0 +1,9 @@
+package options
+
+const (
+ Instance string = "instance"
+)
+
+var SupportedResources = map[string]string{
+ "instance": Instance,
+}

pkg/controllers/options/tagging_controller.go

@@ -1,3 +1,16 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package options
 
 import (
@@ -6,17 +19,29 @@ import (
 )
 
 type TaggingControllerOptions struct {
- Tags map[string]string
+ Tags map[string]string
+ Resources []string
 }
 
 func (o *TaggingControllerOptions) AddFlags(fs *pflag.FlagSet) {
  fs.StringToStringVar(&o.Tags, "tags", o.Tags, "Tags to apply to AWS resources in the tagging controller.")
+ fs.StringArrayVar(&o.Resources, "resources", o.Resources, "AWS resources name to add/remove tags in the tagging controller.")
 }
 
 func (o *TaggingControllerOptions) Validate() error {
  if len(o.Tags) == 0 {
  return fmt.Errorf("--tags must not be empty")
  }
 
+ if len(o.Resources) == 0 {
+ return fmt.Errorf("--resources must not be empty")
+ }
+
+ for _, r := range o.Resources {
+ if _, ok := SupportedResources[r]; !ok {
+ return fmt.Errorf("%s is not a supported resource", r)
+ }
+ }
+
  return nil
 }

pkg/controllers/tagging/tagging_controller.go

@@ -25,6 +25,7 @@ import (
  v1lister "k8s.io/client-go/listers/core/v1"
  cloudprovider "k8s.io/cloud-provider"
  "k8s.io/cloud-provider-aws/pkg/controllers/options"
+ opt "k8s.io/cloud-provider-aws/pkg/controllers/options"
  awsv1 "k8s.io/cloud-provider-aws/pkg/providers/v1"
  "k8s.io/klog/v2"
  "time"
@@ -45,7 +46,7 @@ type TaggingController struct {
  nodeMonitorPeriod time.Duration
 
  // A map presenting the node and whether it currently exists
- currNodes map[string]bool
+ currentNodes map[string]bool
 
  // A map representing nodes that were ever part of the cluster
  totalNodes map[string]*v1.Node
@@ -63,7 +64,8 @@ func NewTaggingController(
  kubeClient clientset.Interface,
  cloud cloudprovider.Interface,
  nodeMonitorPeriod time.Duration,
- tags map[string]string) (*TaggingController, error) {
+ tags map[string]string,
+ resources []string) (*TaggingController, error) {
 
  awsCloud, ok := cloud.(*awsv1.Cloud)
  if !ok {
@@ -76,9 +78,10 @@ func NewTaggingController(
  nodeLister: nodeInformer.Lister(),
  cloud: awsCloud,
  nodeMonitorPeriod: nodeMonitorPeriod,
- currNodes:  make(map[string]bool),
+ currentNodes: make(map[string]bool),
  totalNodes: make(map[string]*v1.Node),
  tags: tags,
+ resources: resources,
  }
  return tc, nil
 }
@@ -97,24 +100,24 @@ func (tc *TaggingController) MonitorNodes(ctx context.Context) {
  klog.Fatalf("error listing nodes: %s", err)
  }
 
- for k := range tc.currNodes {
- tc.currNodes[k] = false
+ for k := range tc.currentNodes {
+ tc.currentNodes[k] = false
  }
 
  var nodesToTag []*v1.Node
  for _, node := range nodes {
- if _, ok := tc.currNodes[node.GetName()]; !ok {
+ if _, ok := tc.currentNodes[node.GetName()]; !ok {
  nodesToTag = append(nodesToTag, node)
  }
 
  tc.totalNodes[node.GetName()] = node
- tc.currNodes[node.GetName()] = true
+ tc.currentNodes[node.GetName()] = true
  }
  tc.tagNodesResources(nodesToTag)
 
  var nodesToUntag []*v1.Node
- for nodeName, existed := range tc.currNodes {
- if existed == false {
+ for nodeName, existed := range tc.currentNodes {
+ if !existed {
  nodesToUntag = append(nodesToUntag, tc.totalNodes[nodeName])
  }
  }
@@ -126,12 +129,18 @@ func (tc *TaggingController) MonitorNodes(ctx context.Context) {
 func (tc *TaggingController) tagNodesResources(nodes []*v1.Node) {
  for _, node := range nodes {
  nodeTagged := false
- nodeTagged = tc.tagEc2Instances(node)
+
+ for _, resource := range tc.resources {
+ switch resource {
+ case opt.Instance:
+ nodeTagged = tc.tagEc2Instances(node)
+ }
+ }
 
  if !nodeTagged {
- // Node tagged unsuccessfully, remove from currNodes
+ // Node tagged unsuccessfully, remove from currentNodes
  // so that we can try later if it still exists
- delete(tc.currNodes, node.GetName())
+ delete(tc.currentNodes, node.GetName())
  }
  }
 }
@@ -161,10 +170,15 @@ func (tc *TaggingController) tagEc2Instances(node *v1.Node) bool {
 func (tc *TaggingController) untagNodeResources(nodes []*v1.Node) {
  for _, node := range nodes {
  nodeUntagged := false
- nodeUntagged = tc.untagEc2Instance(node)
+
+ for _, resource := range tc.resources {
+ if resource == opt.Instance {
+ nodeUntagged = tc.untagEc2Instance(node)
+ }
+ }
 
  if nodeUntagged {
- delete(tc.currNodes, node.GetName())
+ delete(tc.currentNodes, node.GetName())
  }
  }
 }

pkg/controllers/tagging/tagging_controller_test.go

@@ -49,8 +49,8 @@ func Test_NodesJoiningAndLeaving(t *testing.T) {
  },
  },
  taggingController: TaggingController{
- currNodes:  make(map[string]bool),
- totalNodes: make(map[string]*v1.Node),
+ currentNodes: make(map[string]bool),
+ totalNodes:  make(map[string]*v1.Node),
  },
  noOfToBeTaggedNodes: 1,
  totalNodes: 1,
@@ -67,7 +67,7 @@ func Test_NodesJoiningAndLeaving(t *testing.T) {
  },
  },
  taggingController: TaggingController{
- currNodes: map[string]bool{
+ currentNodes: map[string]bool{
  "node0": true,
  },
  totalNodes: map[string]*v1.Node{
@@ -97,7 +97,7 @@ func Test_NodesJoiningAndLeaving(t *testing.T) {
  },
  },
  taggingController: TaggingController{
- currNodes: map[string]bool{
+ currentNodes: map[string]bool{
  "node0": true,
  "node1": true,
  },
@@ -137,7 +137,7 @@ func Test_NodesJoiningAndLeaving(t *testing.T) {
  },
  },
  taggingController: TaggingController{
- currNodes: map[string]bool{
+ currentNodes: map[string]bool{
  "node0": true,
  "node1": true,
  "node2": true,
@@ -187,7 +187,7 @@ func Test_NodesJoiningAndLeaving(t *testing.T) {
  },
  },
  taggingController: TaggingController{
- currNodes: map[string]bool{
+ currentNodes: map[string]bool{
  "node0": true,
  "node1": true,
  "node2": true,
@@ -237,7 +237,7 @@ func Test_NodesJoiningAndLeaving(t *testing.T) {
  },
  },
  taggingController: TaggingController{
- currNodes: map[string]bool{
+ currentNodes: map[string]bool{
  "node0": true,
  },
  totalNodes: map[string]*v1.Node{
@@ -283,9 +283,10 @@ func Test_NodesJoiningAndLeaving(t *testing.T) {
 
  testcase.taggingController.MonitorNodes(ctx)
 
- if len(testcase.taggingController.currNodes) != testcase.noOfToBeTaggedNodes || len(testcase.taggingController.totalNodes) != testcase.totalNodes {
- t.Errorf("currNodes must contain %d element(s), and totalNodes must contain %d element(s).", testcase.noOfToBeTaggedNodes, testcase.totalNodes)
+ if len(testcase.taggingController.currentNodes) != testcase.noOfToBeTaggedNodes || len(testcase.taggingController.totalNodes) != testcase.totalNodes {
+ t.Errorf("currentNodes must contain %d element(s), and totalNodes must contain %d element(s).", testcase.noOfToBeTaggedNodes, testcase.totalNodes)
  }
+
  })
  }
 }

pkg/controllers/tagging/tagging_controller_wrapper.go

@@ -32,7 +32,7 @@ func (tc *TaggingControllerWrapper) StartTaggingControllerWrapper(initContext ap
 func (tc *TaggingControllerWrapper) startTaggingController(ctx context.Context, initContext app.ControllerInitContext, completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) (controller.Interface, bool, error) {
  err := tc.Options.Validate()
  if err != nil {
- klog.Fatal("Tagging controller inputs are not properly set.")
+ klog.Fatalf("Tagging controller inputs are not properly set: %v", err)
  }
 
  // Start the TaggingController
@@ -41,7 +41,8 @@ func (tc *TaggingControllerWrapper) startTaggingController(ctx context.Context,
  completedConfig.ClientBuilder.ClientOrDie(initContext.ClientName),
  cloud,
  completedConfig.ComponentConfig.KubeCloudShared.NodeMonitorPeriod.Duration,
- tc.Options.Tags)
+ tc.Options.Tags,
+ tc.Options.Resources)
 
  if err != nil {
  klog.Warningf("failed to start tagging controller: %s", err)

pkg/providers/v1/aws.go

@@ -1164,7 +1164,7 @@ func (s *awsSdkEC2) DeleteTags(request *ec2.DeleteTagsInput) (*ec2.DeleteTagsOut
  requestTime := time.Now()
  resp, err := s.ec2.DeleteTags(request)
  timeTaken := time.Since(requestTime).Seconds()
- recordAWSMetric("create_tags", timeTaken, err)
+ recordAWSMetric("delete_tags", timeTaken, err)
  return resp, err
 }