test/calico: initial commit

[invidian]

This PR adds initial test for Calico, which checks, that each Node
object in the cluster has associated HostEndpoint Calico object, which
ensures, that GlobalNetworkPolicy objects take effect.

Refs #137.

Signed-off-by: Mateusz Gozdek mateusz@kinvolk.io

NOTE: This PR also includes commits from #209, which should be merged first.

[surajssd]

LGTM

[iaguis]

lgtm

[invidian]

I'll trigger the CI again to make sure the tests are not flaky.

[rata]

Thanks for the tests!

This seems like e2e test and only testing that nodes have an endpoint associated, but not that they are using the right network interface (bond0 in packet case), labels (i.e controller vs worker, etc.) and I guess it increases the time to test quite a lot.

Doesn't a unit test, that just checks the generated yaml for that is way more easier and faster to test? (i.e. test will run in a ~1 second) I think those unit tests are in any case useful, specially to refactor code (if we move from generating the values.yaml helm thingy from terraform to lokoctl), and very easy and fast to run them locally.

Why did you decided to go with e2e tests for this? I see a benefit of doing an e2e test like the one suggested here that will just check if anything is open (in a blackbox fashion), but not sure if doing this test in e2e (instead of unit) gains us much here. But I might be missing something. What was the reasoning for going with a very specific (tightly coupled with calico) e2e test? :)

[invidian]

@rata thanks for the review. I think at this point, testing YAML content would be more complex to implement, so I'd rather not call it "unit" test. In my opinion, the test I implemented provides good balance between the complexity of implementation and the coverage of the functionality. I agree that it doesn't test every aspect of the solution, but at least it should catch the regression which could be caused by the way we create those objects. If Calico or Packet changes the behavior, then we won't notice it, but this should probably be covered by blackbox tests we don't have right now.

[rata]

Oh, it makes sense if the other is more complicated, thanks for the explanation! LGTM

I'd like to keep tests clean and useful, while not taking too much time to run.

Do you think it makes sense to remove this test once we have the blackbox test? Maybe we can point it out in the blackbox issue. And, hopefully, have unit tests later for things that will be way faster?

[invidian]

Do you think it makes sense to remove this test once we have the blackbox test? Maybe we can point it out in the blackbox issue. And, hopefully, have unit tests later for things that will be way faster?

I guess that depends on how do we implement blackbox tests.

[invidian]

Except regular CI flakiness, the test itself seems stable.

[surajssd]

LGTM