Deprecate `DisableVPCCNI` in `AWSManagedControlPlane`

[richardcase]

trafficstars

/kind feature /area provider/eks /priority important-longterm /triage accepted /help

Describe the solution you'd like With #3374 we are introducing a new struct for settings that relate to the VPC CNI (a.k.a aws-node). We should add a Disable field to this and start deprecation of DisableVPCCNI.

Any code that relies on DisableVPCCNI should be changed to use a field on the managed control plane scope that can internally get the values from DisableVPCCNI or VPCCNI.Disable.

We will also probably need to update the admission controller to:

• report that DisableVPCCNI is deprecated
• check if the values of DisableVPCCNI & VPCCNI.Disable are set to something different

Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]

Environment:

• Cluster-api-provider-aws version:
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[k8s-ci-robot]

@richardcase: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to this:

/kind feature /area provider/eks /priority important-longterm /triage accepted /help

Describe the solution you'd like With #3374 we are introducing a new struct for settings that relate to the VPC CNI (a.k.a aws-node). We should add a Disable field to this and start deprecation of DisableVPCCNI.

Any code that relies on DisableVPCCNI should be changed to use a field on the managed control plane scope that can internally get the values from DisableVPCCNI or VPCCNI.Disable.

We will also probably need to update the admission controller to:

• report that DisableVPCCNI is deprecated
• check if the values of DisableVPCCNI & VPCCNI.Disable are set to something different

Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]

Environment:

• Cluster-api-provider-aws version:
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[charlie-haley]

I'm happy to take a look at this once #3374 has been merged into master

/assign @charlie-haley

[charlie-haley]

@richardcase do you also think it would make sense to do this https://github.com/kubernetes-sigs/cluster-api-provider-aws/issues/3544 as part of the same PR?

[richardcase]

@richardcase do you also think it would make sense to do this #3544 as part of the same PR?

I'd be tempted to do them as 2 separate PRs. Only because the e2e tests can take a while to get working. But if you want to do them as a single PR then thats good as well.

It would be great to improve the e2e coverage on the EKS side, so if you are keen in this area and if would be useful i could do a a brain dump on developing e2e tests in CAPA.

[charlie-haley]

@richardcase do you also think it would make sense to do this #3544 as part of the same PR?

I'd be tempted to do them as 2 separate PRs. Only because the e2e tests can take a while to get working. But if you want to do them as a single PR then thats good as well.

It would be great to improve the e2e coverage on the EKS side, so if you are keen in this area and if would be useful i could do a a brain dump on developing e2e tests in CAPA.

Sounds good, I can take a look at the e2e tests once these API changes are in, I'll reach out when I start work on them, thanks!

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten