Changes to enable ST tests of golang calicoctl #163
Conversation
|
I think it's better to use uncompressed container images (I think I saw that you'd changed some to use .tgz) |
|
I (and @fasaxc ) thought that compressed was better because of the problems we'd had recently with semaphore or circle running out of disk space. I don't feel strongly either way though. What's your reason for uncompressed being better? |
| pformat(DeepDiff(thing1, thing2), indent=2) | ||
|
|
||
| @staticmethod | ||
| def writeyaml(filename, data): |
There was a problem hiding this comment.
Quick doc string here and below would be super
| def __init__(self, name, start_calico=True, dind=True, | ||
| additional_docker_options="", | ||
| post_docker_commands=["docker load -i /code/calico-node.tar", | ||
| "docker load -i /code/busybox.tar"], | ||
| post_docker_commands=["docker load -i /code/calico-node.tgz", |
There was a problem hiding this comment.
I would like to understand Toms objection here. Could we actually allow both?
| def wrapped(*args, **kwargs): | ||
| try: | ||
| return fn(*args, **kwargs) | ||
| except KeyboardInterrupt: | ||
| raise | ||
| except Exception as e: | ||
| if (os.getenv("DEBUG_FAILURES") != None and | ||
| os.getenv("DEBUG_FAILURES").lower() == "true"): | ||
| if (os.getenv("DEBUG_FAILURES") is not None and os.getenv( |
There was a problem hiding this comment.
i think this splitting is actually harder to read
| @@ -93,7 +95,11 @@ def _get_ping_function(self, ip): | |||
| "-c", "1", # Number of pings | |||
| "-W", "1", # Timeout for each ping | |||
| ip, | |||
| ] | |||
| # "hping", | |||
There was a problem hiding this comment.
What is this here for? Shouldn't have commented out code checked into master
| """ | ||
| # test_string = "hello" | ||
| args = [ | ||
| # "echo", test_string, "|", # send data to responder |
There was a problem hiding this comment.
Again - shouldn't have this in the checked-in code
| logger.error("TEST FAILED:\n%s\nEntering DEBUG mode." | ||
| % e.message) | ||
| pdb.set_trace() | ||
| else: | ||
| raise e | ||
|
|
||
| return wrapped | ||
|
|
||
|
|
There was a problem hiding this comment.
check_bird_status()
This needs updating to run calicoctl node status rather than calicoctl status
Do we have any tests for that?
There was a problem hiding this comment.
Not yet - you only added that last night ;)
|
For the mass merge to master I think we want to remove the "go/python" calicoctl option - just assume it's go. |
|
Switched back to uncompressed images. |
lwr20
changed the title
| :param data: string, the data to put inthe file | ||
| :return: Return code of execute operation. | ||
| """ | ||
| return self.execute("cat << EOF > %s\n%s" % (filename, data)) |
There was a problem hiding this comment.
I think you need another EOF at the end of this string to terminate, yeah?
There was a problem hiding this comment.
It's weird, but it seems to work. Maybe docker adds the EOF or something. Since we're rushed I think it's fine as is.
Needed for tests in projectcalico/calicoctl#1252 to work.