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

refine ENI tagging logic #1482

Merged
merged 1 commit into from
Jun 7, 2021
Merged

refine ENI tagging logic #1482

merged 1 commit into from
Jun 7, 2021

Conversation

M00nF1sh
Copy link
Contributor

@M00nF1sh M00nF1sh commented Jun 4, 2021

What type of PR is this?
/kind cleanup
/kind feature

Which issue does this PR fix:
N/A

What does this PR do / Why do we need it:
This PR did following change:

  1. All ENIs created by IPAM-D will be tagged with all desired tags on creation.
  2. All ENIs managed by IPAM-D will be tagged with all desired tags if not already tagged with these tags. Other tags on ENI will be kept as it is.
    • Trunk ENI is excluded, as it's lifecycle is managed by vpc-resource-controller.
    • Since we tag ENIs on ENI creation, this backfill logic will only trigger for below cases:
      • ENIs created by previous versions of ENI and the desired tag set changed.
      • ENIs attached to instances from external source without the node.k8s.amazonaws.com/no_manage tag.
  3. The desired tag set is:
    • node.k8s.amazonaws.com/instance_id: <instance_id>
    • cluster.k8s.amazonaws.com/name: <cluster-name> if CLUSTER_NAME envVar is specified.
    • additional tags specified if ADDITIONAL_ENI_TAGS envVar is specified.

This PR gives us below benefit, which allow customers to use a scoped down IAM policy

  • If ENI is configured with CLUSTER_NAME environment variable, ensure ENI is tagged with clusterName tag on creation.
  • If ENI is configured with CLUSTER_NAME environment variable, ensure managed ENIs have clusterName tag backfilled.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

Does this change require updates to the CNI daemonset config files to work?:

Does this PR introduce any user-facing change?:

IPAMD now tag ENIs with desired tags on ENI creation and backfill desired tags for ENIs attached to node.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jayanthvn jayanthvn self-requested a review June 4, 2021 17:29
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
@M00nF1sh M00nF1sh force-pushed the secure_policy branch 10 times, most recently from 4e0f62f to b247e1c Compare June 4, 2021 22:47
@M00nF1sh M00nF1sh closed this Jun 5, 2021
@M00nF1sh M00nF1sh reopened this Jun 5, 2021
@M00nF1sh M00nF1sh force-pushed the secure_policy branch 4 times, most recently from ea85ff7 to fc3a0cc Compare June 5, 2021 01:15
1. All ENIs created by IPAM-D will be tagged with all desired tags on creation.
2. All ENIs managed by IPAM-D will be tagged with all desired tags if not already tagged with these tags. Other tags on ENI will be kept as it is.
   1. Trunk ENI is excluded, as it's lifecycle is managed by vpc-resource-controller.
   2. Since we tag ENIs on ENI creation, this backfill logic will only trigger for below cases:
       * ENIs created by previous versions of ENI and the desired tag set changed.
       * ENIs attached to instances from external source without the node.k8s.amazonaws.com/no_manage tag.
3. The desired tag set is:
   * node.k8s.amazonaws.com/instance_id: <instance_id>
   * cluster.k8s.amazonaws.com/name: <cluster-name> if CLUSTER_NAME envVar is specified.
   * additional tags specified if ADDITIONAL_ENI_TAGS envVar is specified.
@M00nF1sh M00nF1sh merged commit 9839583 into aws:master Jun 7, 2021
M00nF1sh added a commit to M00nF1sh/amazon-vpc-cni-k8s that referenced this pull request Jun 7, 2021
1. All ENIs created by IPAM-D will be tagged with all desired tags on creation.
2. All ENIs managed by IPAM-D will be tagged with all desired tags if not already tagged with these tags. Other tags on ENI will be kept as it is.
   1. Trunk ENI is excluded, as it's lifecycle is managed by vpc-resource-controller.
   2. Since we tag ENIs on ENI creation, this backfill logic will only trigger for below cases:
       * ENIs created by previous versions of ENI and the desired tag set changed.
       * ENIs attached to instances from external source without the node.k8s.amazonaws.com/no_manage tag.
3. The desired tag set is:
   * node.k8s.amazonaws.com/instance_id: <instance_id>
   * cluster.k8s.amazonaws.com/name: <cluster-name> if CLUSTER_NAME envVar is specified.
   * additional tags specified if ADDITIONAL_ENI_TAGS envVar is specified.
M00nF1sh added a commit to M00nF1sh/amazon-vpc-cni-k8s that referenced this pull request Jun 7, 2021
1. All ENIs created by IPAM-D will be tagged with all desired tags on creation.
2. All ENIs managed by IPAM-D will be tagged with all desired tags if not already tagged with these tags. Other tags on ENI will be kept as it is.
   1. Trunk ENI is excluded, as it's lifecycle is managed by vpc-resource-controller.
   2. Since we tag ENIs on ENI creation, this backfill logic will only trigger for below cases:
       * ENIs created by previous versions of ENI and the desired tag set changed.
       * ENIs attached to instances from external source without the node.k8s.amazonaws.com/no_manage tag.
3. The desired tag set is:
   * node.k8s.amazonaws.com/instance_id: <instance_id>
   * cluster.k8s.amazonaws.com/name: <cluster-name> if CLUSTER_NAME envVar is specified.
   * additional tags specified if ADDITIONAL_ENI_TAGS envVar is specified.
M00nF1sh added a commit that referenced this pull request Jun 7, 2021
1. All ENIs created by IPAM-D will be tagged with all desired tags on creation.
2. All ENIs managed by IPAM-D will be tagged with all desired tags if not already tagged with these tags. Other tags on ENI will be kept as it is.
   1. Trunk ENI is excluded, as it's lifecycle is managed by vpc-resource-controller.
   2. Since we tag ENIs on ENI creation, this backfill logic will only trigger for below cases:
       * ENIs created by previous versions of ENI and the desired tag set changed.
       * ENIs attached to instances from external source without the node.k8s.amazonaws.com/no_manage tag.
3. The desired tag set is:
   * node.k8s.amazonaws.com/instance_id: <instance_id>
   * cluster.k8s.amazonaws.com/name: <cluster-name> if CLUSTER_NAME envVar is specified.
   * additional tags specified if ADDITIONAL_ENI_TAGS envVar is specified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants