search

projectcalico / api

Calico API
Apache License 2.0
17 stars 26 forks source link

AWSSrcDstCheckOptionDisable inconsistency #17

Closed frozenprocess closed 3 years ago

frozenprocess commented 3 years ago

I feel like there is an issue in our API code (AWSSrcDstCheckOptionDisable).

Defined value in code is Disable. https://github.com/projectcalico/api/blob/ae822ba06c23ee9282f9cb104c1d3c9d371782da/pkg/apis/projectcalico/v3/felixconfig.go#L60 But in line 375 we have commented Disabled as correct value. https://github.com/projectcalico/api/blob/ae822ba06c23ee9282f9cb104c1d3c9d371782da/pkg/apis/projectcalico/v3/felixconfig.go#L375-L377

Expected Behavior

Felix should become ready when using Disable as a value for aws source destination check.

Current Behavior

Calico-node goes into a loop, outptuing the following warning when value is set to Disable. 

2021-07-28 18:04:29.029 [WARNING][80] felix/health.go 188: Reporter is not ready. name="aws-source-destination-check"
2021-07-28 18:04:32.586 [WARNING][80] felix/health.go 188: Reporter is not ready. name="aws-source-destination-check"
2021-07-28 18:04:32.587 [WARNING][80] felix/health.go 154: Health: not ready

This warning ultimately results in an Error message

2021-07-28 17:54:35.475 [ERROR][80] felix/route_table.go 931: Failed to get link attributes error=interface not present ifaceRegex="^vxlan.calico$" ipVersion=0x4

Possible Solution

Feels like instead of Disable we should use Disabled in following locations.

https://github.com/projectcalico/api/blob/ae822ba06c23ee9282f9cb104c1d3c9d371782da/pkg/apis/projectcalico/v3/felixconfig.go#L60

https://github.com/projectcalico/api/blob/ae822ba06c23ee9282f9cb104c1d3c9d371782da/pkg/apis/projectcalico/v3/felixconfig.go#L375-L377

https://github.com/projectcalico/calico/blob/4c28dab0e5fe251d07459f8386165a0ae58ec285/_includes/charts/calico/templates/calico-node.yaml#L335

https://github.com/projectcalico/calico/blob/51f37983a1d90e4976ca65b24c2e7b33c1183f98/_includes/charts/calico/crds/kdd/crd.projectcalico.org_felixconfigurations.yaml#L44-L51

https://github.com/tigera/operator-cloud/blob/a5388205db2413c36b26c53664150b96611fdd9d/pkg/controller/migration/convert/felix_vars_test.go#L85

Steps to Reproduce (for bugs)

  1. apply calico-vxlan.yaml to your aws self-managed cluster

Context

I was trying to install Calico using calico-vxlan.yaml file.

Your Environment

CC: @caseydavenport

caseydavenport commented 3 years ago

Yep, looks like the comment / docstring is wrong for the field.