Make src/dst checks configurable on awsmachineclass

[DockToFuture]

What would you like to be added:
To support calico's "CrossSubnet" mode on gardener clusters for provider type aws the src and dst checks (https://docs.aws.amazon.com/vpc/latest/userguide/VPC_NAT_Instance.html#EIP_Disable_SrcDestCheck) have to be disabled on machine network interface level. Therefore I would like to expose a field in the awsmachineclass (https://github.com/gardener/machine-controller-manager/blob/master/pkg/apis/machine/v1alpha1/aws_machineclass_types.go#L179) which contains a boolean value which describes if the src/dst checks on the interface level of the machines are enabled or disabled.
I would suggest something like

// Describes a network interface.
// Please also see https://docs.aws.amazon.com/goto/WebAPI/ec2-2016-11-15/MachineAWSNetworkInterfaceSpecification
type AWSNetworkInterfaceSpec struct { 
   // If set to false, source and destination checks are disabled, default is true
   SrcAndDstChecksEnabled: bool `json:"srcAndDstChecksEnabled,omitempty"`
}

WDYT?
/cc: @zanetworker

[prashanth26]

I would suggest having this type as *bool as this a newly added field and in the API. And nil value would mimic the current behavior and only when set overrides it.

With this update, I don't think (need to verify) it will update any existing VMs as we do not update the VMs once created. However, newly created VMs will have this feature enabled.

[hardikdr]

/add kind/api-change
/add kind/feature
/add platform/aws

[hardikdr]

This is related to : gardener/machine-controller-manager#441 .

• This was proposed, discussed and we had not implemented due to other dependencies.
• cc @zuzzas

We have an atomic Create() call, and I believe the src-dest check can be enabled only via a separate ModifyInstanceAttribute() call.

• To have a reliable solution, we need better state-management, and also in-place updates to avoid roll-outs of machines.

How critical is the need at the moment, I'd set the normal priority, for now, please change if otherwise.

[hardikdr]

/kind discussion

[zuzzas]

If you use AWS cloud-controller-manager (separately or as a legacy part of kube-controller-manager), it'll set this option on every Node. That's why we aren't interested in this feature being part of machine-controller-manager.

[zanetworker]

@zuzzas correct me if I am wrong, but isn't this the case only if the cloud-controller is set to configure routes? otherwise, src/destination checks are enabled by default. I guess it would be helpful to have more control over when it is enabled / disabled.

[zuzzas]

@zanetworker
That is correct.

[vpnachev]

The node roll out can be forced by the gardener-extension-provider-aws here https://github.com/gardener/gardener-extension-provider-aws/blob/09bca6c28530090af970a5aeafbd29fef3b26c21/pkg/controller/worker/machines.go#L307 by including this field in the data for computing the hash.

What is not clear to me is whether provider-aws will directly manage this field or the calico extension will implement a mutating webhook to adjust this field?

[zanetworker]

@vpnachev yea that was the intention. It would have been great though if you can update without rolling the VM, as this procedure is not disruptive and can be done while the VM is running.

Eventually, this is needed for Network config updates, so waiting for the maintenance and the VMs to be rolled can be a bit challenging.

[prashanth26]

As discussed overcall. The final approach would be

• Introduce this flag on MCM provider AWS - Creation of the machine will change to a two-step process
• Update the provider-aws (worker) extension to use this flag by default from k8s > v1.22
• Enable the cross subnet flag post k8s > v1.22 by default as well.

[vpnachev]

Why do you want to use flag when you can use field in the MachineClass?
This way you can have worker pools with different settings controller by exactly the same instance of the MCM.

[prashanth26]

Why do you want to use flag when you can use field in the MachineClass?
This way you can have worker pools with different settings controller by exactly the same instance of the MCM.

Sorry it was a typo. I mean't to keep it as a field