Closed abutcher closed 6 years ago
This looks great as far as I can see.
Could you add some unit tests? We mock the ELB registration call here: https://github.com/openshift/cluster-operator/blob/master/pkg/controller/awselb/aws_elb_controller_test.go#L162 So you could setup the test struct to optionally let the test specify an ELB registration error, and then validate there's a condition set as we'd expect. (both for success and failure)
/lgtm
dgoodwin
commented
6 years ago dgoodwin
commented
6 years ago
This looks great as far as I can see.
Could you add some unit tests? We mock the ELB registration call here: https://github.com/openshift/cluster-operator/blob/master/pkg/controller/awselb/aws_elb_controller_test.go#L162 So you could setup the test struct to optionally let the test specify an ELB registration error, and then validate there's a condition set as we'd expect. (both for success and failure)