kube-ingress-aws-controller icon indicating copy to clipboard operation
kube-ingress-aws-controller copied to clipboard

Published 20 hours ago • verified publisher icon zalando-incubator

Add `Legacy` `target-access-mode` to enable upgrade from pre `0.12.17` version

Open AlexanderYastrebov opened this issue 3 years ago • 4 comments

Before version 0.12.17 loadbalancer target group target type was not specified and defaulted to instance in CloudFormation, see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-targettype

PR #461 introduced AWS CNI mode and configured target group target type either to ip or instance.

Changing target type from unset to instance in Cloudformation triggers target group re-creation which makes it impossible to upgrade from pre 0.12.17 without downtime.

This change:

  • makes target-access-mode flag required to force users to choose proper value
  • renames HostPort option of target-access-mode into InstanceFilter
  • introduces a new Legacy option that does not set target type and thus enables upgrade from pre 0.12.17.

Fixes #507

Signed-off-by: Alexander Yastrebov [email protected]

AlexanderYastrebov avatar Jul 04 '22 20:07 AlexanderYastrebov

Are you still working on this @AlexanderYastrebov ?

jbilliau-rcd avatar Jul 23 '22 16:07 jbilliau-rcd

@jbilliau-rcd Hi, we need some effort to push it forward, currently I am overwhelmed by the backlog

AlexanderYastrebov avatar Jul 25 '22 13:07 AlexanderYastrebov

hey @AlexanderYastrebov , just checking in, I know you prob swamped. This still in limbo?

jbilliau-rcd avatar Sep 20 '22 15:09 jbilliau-rcd

@jbilliau-rcd soon (~2nd week of October IIRC) we will have more time, because of internal code freeze. Sorry for the delay. :(

szuecs avatar Sep 20 '22 18:09 szuecs

@szuecs hey, just checking in; how we looking?

jbilliau-rcd avatar Oct 25 '22 20:10 jbilliau-rcd

Let's ask @AlexanderYastrebov

szuecs avatar Oct 25 '22 20:10 szuecs

hey @AlexanderYastrebov , looks like the only thing holding this up is some code coverage tests?

jbilliau-rcd avatar Nov 02 '22 15:11 jbilliau-rcd

@jbilliau-rcd this is not required. Can you open the PR as non-draft, please?

szuecs avatar Nov 02 '22 20:11 szuecs

@szuecs it's not my PR, I have no access to do that. Also, I'm not sure if @AlexanderYastrebov if fully done or not, maybe he left it in draft for a reason, was still vetting stuff out? Not sure.

jbilliau-rcd avatar Nov 02 '22 20:11 jbilliau-rcd

@jbilliau-rcd oh, my bad will try to ask him to finish it up.

szuecs avatar Nov 03 '22 08:11 szuecs

Looks like this is all ready to go? @szuecs are you someone who can review and merge? I appreciate the work btw; we have 200 clusters using this controller and so this is a HUGE burden off our shoulders since we have to upgrade to avoid the BoundServiceAccountToken expiration issue.

jbilliau-rcd avatar Nov 03 '22 15:11 jbilliau-rcd

:+1:

szuecs avatar Nov 03 '22 16:11 szuecs

:+1:

AlexanderYastrebov avatar Nov 07 '22 15:11 AlexanderYastrebov