v1.9 backports 2021-03-12 #15331

kaworu · 2021-03-12T11:23:43Z

azure: Add the correct JSON tag to GatewayIP field in CiliumNode CRD #15182 -- azure: Add the correct JSON tag to GatewayIP field in CiliumNode CRD (@AnishShah)
- NOTE: partially backported, see v1.9 backports 2021-03-12 #15331 (comment)
doc: Document minimal version of AWS CNI in chaining mode #15304 -- doc: Document minimal version of AWS CNI in chaining mode (@tgraf)
node: Fix CIDR comparison when updating routes #15263 -- node: Fix CIDR comparison when updating routes (@brb)
azure: populate ApplicationSecurityGroups field in the new IPConfigurations #15194 -- azure: populate ApplicationSecurityGroups field in the new IPConfigurations (@AnishShah)
ipam: Fix ENI routing for secondary CIDRs #15303 -- ipam: Fix ENI routing for secondary CIDRs (@gandro)

Skipped:

endpoint: Fix the initialization of endpoints' datapath configs #15228 -- endpoint: Fix the initialization of endpoints' datapath configs (@pchaigno)
- lacking context to resolve the conflict around the RequireRouting disable.

Once this PR is merged, you can update the PR labels via:

$ for pr in 15182 15304 15263 15194 15303; do contrib/backporting/set-labels.py $pr done 1.9; done

[ upstream commit ba06970 ] The incorrect JSON tag "-" was added to GatewayIP field in commit 8de6dc5 that was causing GatewayIP field to not get populated in CiliumNode CRD. Signed-off-by: Anish Shah <anishshah@google.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

kaworu · 2021-03-12T11:30:45Z

@ti-mo could you take a close look at the backport of #15182 (azure: Add the correct JSON tag to GatewayIP field in CiliumNode CRD) please?

I had to resolve some conflicts and I lack context about how pkg/k8s/apis/cilium.io/v2/client/bindata.go is generated. Specifically: does it matter that the vmssName member of AzureInterface is named differently between v1.9 (VMSSName) and master (vmssName) or is it only a public/private scope change without impact?

kaworu · 2021-03-12T11:31:01Z

test-backport-1.9

AnishShah · 2021-03-12T15:58:13Z

The second commit of #15182 doesn't need backport I think. @aanm

aanm · 2021-03-12T16:31:58Z

The wedding commit of #15182 doesn't need backport I think. @aanm

If you mean the one with the tittle "azure: Add Gateway field to AzureInterface and deprecate GatewayIP field" I agree, we need to drop that one.

[ upstream commit a3fea6b ] AWS CNI 1.7.9 is fixed to be compatible with Cilium. Signed-off-by: Thomas Graf <thomas@cilium.io> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

[ upstream commit 508b2de ] Previously, the CIDR comparison didn't take into account the CIDR mask. So, if IP didn't change, but the mask did, the comparison would have not detected the change. The result of it was that old routes could not be removed. Fixes: c7a83bf ("datapath/linux: New scalable routing layer via Linux datapath implementation") Signed-off-by: Sebastian Wicki <sebastian@isovalent.com> Signed-off-by: Martynas Pumputis <m@lambda.lt> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

[ upstream commit 272c1fe ] Azure wants all IPConfigurations to have the same ApplicationSecurityGroups. So if the primary IPConfiguration is already assigned an ApplicationSecurityGroup, adding a new IPConfiguration without any ApplicationSecurityGroup fails. So we should populate ApplicationSecurityGroups field that is the same as ASG of other IPConfiguration. Signed-off-by: Anish Shah <anishshah@google.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

kaworu · 2021-03-15T08:52:34Z

@AnishShah thank you, I've dropped "azure: Add Gateway field to AzureInterface and deprecate GatewayIP field" from this PR, please take another look!

/cc @aanm

[ upstream commit b447fde ] Backporting note: replaced aws.ToString() by aws.StringValue(), see https://aws.github.io/aws-sdk-go-v2/docs/migrating/#pointer-parameters A VPC on EC2 can have secondary CIDRs which are routable within the VPC. Subnets which are used in Cilium's ENI IPAM mode might be derived from these secondary CIDRs, therefore we must also install routes for these secondary CIDRs. This commit ensures that we populate the `CiliumNode.Status.ENI.ENIS[].VPC.CIDRs` field with these secondary CIDRs if present. The IPAM code on the agent is already set up to install routes for these additional CIDRs [1], but since this field was never populated, the rules were also missing. Therefore, this fixes a bug where routes were missing in ENI IPAM mode, causing arbitrary connecitivty issues. With this commit, routes are only added for CIDRs which are present when the IP is allocated. A subsequent PR will add the functionality to update the routes dynamically in case CIDRs are added or removed from a VPC. [1] https://github.com/cilium/cilium/blob/2110b11c989fe7ef8c7d9c5510c53a55cdaaa54c/pkg/ipam/crd.go#L488 Signed-off-by: Sebastian Wicki <sebastian@isovalent.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

kaworu · 2021-03-15T09:10:04Z

@gandro this CI failure seems to be related to this patch. In v1.9 we have aws-sdk-go-v2@v0.24.0 vs aws-sdk-go-v2@v1.2.0
in master.

From https://aws.github.io/aws-sdk-go-v2/docs/migrating/#pointer-parameters

When upgrading your application from AWS SDK for Go V1 to AWS SDK for Go V2, you must migrate usage of the helpers for converting from the pointer types to the non-pointer variants. For example, aws.StringValue must be updated to aws.ToString.

So I just s/ToString/StringValue/, please take a look.

ti-mo · 2021-03-15T10:50:41Z

@kaworu Correct, the struct members in azure/types/types.go were lowercased in 7cd51b6. Looks like we could backport that change as well, but might be safer to hold off since it's relatively cross-cutting.

Can't provide any context on bindata generation, sorry. ☹️ Do we generate bindata in CI and check if it dirties the repo?

gandro

My commit looks good! Thanks for fixing!

kaworu · 2021-03-15T14:44:26Z

@ti-mo thanks for taking a look, @AnishShah and @aanm both commented about #15182 so hopefully they can comment also on the bindata stuff.

kaworu · 2021-03-15T14:45:05Z

test-backport-1.9

AnishShah · 2021-03-15T17:11:19Z

I think we do check if the any changes in bindata dirties the repo. It is "Go-related checks / generate-api". But I'm not sure.

kaworu · 2021-03-16T08:58:01Z

test-backport-1.9

kaworu · 2021-03-19T11:22:20Z

test-1.12-netnext

kaworu · 2021-03-19T20:10:38Z

I think we do check if the any changes in bindata dirties the repo. It is "Go-related checks / generate-api". But I'm not sure.

@AnishShah FWIW I've checked and your statement is correct, thanks for the hint!

cilium/.github/workflows/go-check.yaml

Line 95 in 9fa8b2d

contrib/scripts/check-k8s-code-gen.sh

cilium/contrib/scripts/check-k8s-code-gen.sh

Lines 16 to 27 in 9fa8b2d

    
           # Generate all files 
        
           make generate-k8s-api manifests go-bindata 
        
           # Check for diff 
        
           diff="$(git diff)" 
        
           if [ -n "$diff" ]; then 
        
           	echo "Ungenerated deepcopy source code:" 
        
           	echo "$diff" 
        
           	echo "Please run make generate-k8s-api and submit your changes" 
        
           	exit 1 
        
           fi

cilium/Makefile

Lines 422 to 426 in 9fa8b2d

    
           go-bindata: $(K8S_CRD_FILES) 
        
           	@$(ECHO_GEN) $@ 
        
           	cd "./vendor/github.com/go-bindata/go-bindata/v3/go-bindata" && \ 
        
           		$(GO_BINDATA) -o $(ROOT_DIR)/pkg/k8s/apis/cilium.io/v2/client/bindata.go \ 
        
           		$(K8S_CRD_FILES)

brb

👍 for my changes, thanks.

kaworu requested a review from a team as a code owner March 12, 2021 11:23

kaworu added backport/1.9 kind/backports This PR provides functionality previously merged into master. labels Mar 12, 2021

kaworu requested review from brb, gandro and tgraf March 12, 2021 11:25

maintainer-s-little-helper bot assigned brb, gandro and tgraf and unassigned brb Mar 12, 2021

kaworu requested a review from ti-mo March 12, 2021 11:26

maintainer-s-little-helper bot assigned ti-mo Mar 12, 2021

kaworu force-pushed the pr/v1.9-backport-2021-03-12 branch from 2f6c1b6 to ebfd318 Compare March 15, 2021 08:50

tgraf and others added 3 commits March 15, 2021 09:50

doc: Document minimal version of AWS CNI in chaining mode

4f704b3

[ upstream commit a3fea6b ] AWS CNI 1.7.9 is fixed to be compatible with Cilium. Signed-off-by: Thomas Graf <thomas@cilium.io> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

tgraf approved these changes Mar 15, 2021

View reviewed changes

kaworu force-pushed the pr/v1.9-backport-2021-03-12 branch from ebfd318 to 6d17d1d Compare March 15, 2021 09:10

gandro approved these changes Mar 15, 2021

View reviewed changes

kaworu mentioned this pull request Mar 15, 2021

v1.8 backports 2021-03-12 #15329

Merged

jrajahalme approved these changes Mar 15, 2021

View reviewed changes

pchaigno unassigned gandro and tgraf Mar 16, 2021

ti-mo approved these changes Mar 19, 2021

View reviewed changes

kaworu unassigned ti-mo Mar 19, 2021

brb approved these changes Mar 22, 2021

View reviewed changes

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 22, 2021

nebril merged commit 668aed7 into cilium:v1.9 Mar 22, 2021

kaworu deleted the pr/v1.9-backport-2021-03-12 branch March 22, 2021 16:18

joestringer mentioned this pull request Apr 20, 2021

Prepare for release v1.9.6 #15795

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v1.9 backports 2021-03-12 #15331

v1.9 backports 2021-03-12 #15331

kaworu commented Mar 12, 2021 •

edited

kaworu commented Mar 12, 2021

kaworu commented Mar 12, 2021

AnishShah commented Mar 12, 2021 •

edited

aanm commented Mar 12, 2021

kaworu commented Mar 15, 2021

kaworu commented Mar 15, 2021

ti-mo commented Mar 15, 2021

gandro left a comment

kaworu commented Mar 15, 2021

kaworu commented Mar 15, 2021

AnishShah commented Mar 15, 2021

kaworu commented Mar 16, 2021

kaworu commented Mar 19, 2021

kaworu commented Mar 19, 2021 •

edited

brb left a comment

v1.9 backports 2021-03-12 #15331

v1.9 backports 2021-03-12 #15331

Conversation

kaworu commented Mar 12, 2021 • edited

kaworu commented Mar 12, 2021

kaworu commented Mar 12, 2021

AnishShah commented Mar 12, 2021 • edited

aanm commented Mar 12, 2021

kaworu commented Mar 15, 2021

kaworu commented Mar 15, 2021

ti-mo commented Mar 15, 2021

gandro left a comment

Choose a reason for hiding this comment

kaworu commented Mar 15, 2021

kaworu commented Mar 15, 2021

AnishShah commented Mar 15, 2021

kaworu commented Mar 16, 2021

kaworu commented Mar 19, 2021

kaworu commented Mar 19, 2021 • edited

brb left a comment

Choose a reason for hiding this comment

kaworu commented Mar 12, 2021 •

edited

AnishShah commented Mar 12, 2021 •

edited

kaworu commented Mar 19, 2021 •

edited