docker-archive / deploykit

A toolkit for creating and managing declarative, self-healing infrastructure.
Apache License 2.0
2.25k stars 262 forks source link

Support case insensitivity in the protocols of the load balancer ingress controller #770

Closed ericvn closed 6 years ago

ericvn commented 6 years ago

Today, the controller will cycle a route if the protocol is returned with a different case than what was specified during the 'route add'.

An easy recreate of this is to use the example1.yml which creates routes using lower case protocols. If you change the simulator LB to upper case the return in the route ls, the controller will repeatedly remove and add the route:

INFO[11-07|16:17:28] Listeners to sync with L4:               module=controller/ingress desired="[{Port:8080 Protocol:http LoadBalancerPort:80 LoadBalancerProtocol:http Certificate:<nil>}]" fn=github.com/docker/infrakit/pkg/controller/ingress.configureL4
INFO[11-07|16:17:28] listeners to create:                     module=controller/ingress list="[{Port:8080 Protocol:http LoadBalancerPort:80 LoadBalancerProtocol:http Certificate:<nil>}]" fn=github.com/docker/infrakit/pkg/controller/ingress.configureL4
INFO[11-07|16:17:28] listeners to change:                     module=controller/ingress list=[] fn=github.com/docker/infrakit/pkg/controller/ingress.configureL4
INFO[11-07|16:17:28] listeners to remove:                     module=controller/ingress list="[{Port:8080 Protocol:HTTP LoadBalancerPort:80 LoadBalancerProtocol:http Certificate:<nil>}]" fn=github.com/docker/infrakit/pkg/controller/ingress.configureL4
chungers commented 6 years ago

It turns out that this case sensitivity was due to improper parsing of the spec json/yaml -- where lower-case values of protocol got converted into string by the default unmarshaller in Go. Adding proper MarshalJSON/UnmarshalJSON methods on the Protocol struct should fix it - by doing the toUpper case conversion during parsing of the input.