Use atomic ints for namespaces #113
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's always been a weird quirk that you need to specify a unique namespace string when deploying containers. This is for parallel tests such that if 2 tests deploy BlueprintAlice they get unique container names. This PR changes the API so that we just increment an atomic counter and use that as the namespace.
anoadragon453
approved these changes
May 14, 2021
kegsay
added a commit
that referenced
this pull request
Jul 20, 2021
This is the beginning of the work for #119. - Add the concept of `PackageNamespace` to `config.Complement`. Set to `""` for now. - Update all filters to look for a matching package namespace when creating blueprints and containers. - Cleanup `docker.Builder` to read from the config more and from copies of config fields less. This change should be invisible to any existing Complement users. Background: Previously, Complement assumed that each container could be uniquely identified by the combination of `deployment_namespace + blueprint_name + hs_name`. For example, if you were running `BlueprintAlice` then a unique string might look like `5_alice_hs1` where `5` is an atomic, monotonically increasing integer incremented when `Deploy()` is called (see #113 for more info). In a parallel by default world this is no longer true because the `deployment_namespace` is not shared between different test processes. This means we cannot co-ordinate non-clashing namespaces like before. Instead, we will bring in another namespace for the test process (which in #119 will be on a per-package basis, hence the name `PackageNamespace`). As of this PR, literally everything Complement makes (images, containers, networks, etc) are prefixed with this package namespace, which allows multiple complement instances to share the same underlying docker daemon, with caveats: - Creating CA certificates will race and needs a lockfile to prevent 2 processes trying to create the certificate at the same time. - Complement federation servers cannot run together due to trying to bind to `:8448` at the same time. That being said, this PR should enable the parallelisation of a number of CS API only tests, which will come in another PR.
kegsay
added a commit
that referenced
this pull request
Jul 21, 2021
* Introduce the concept of a package namespace This is the beginning of the work for #119. - Add the concept of `PackageNamespace` to `config.Complement`. Set to `""` for now. - Update all filters to look for a matching package namespace when creating blueprints and containers. - Cleanup `docker.Builder` to read from the config more and from copies of config fields less. This change should be invisible to any existing Complement users. Background: Previously, Complement assumed that each container could be uniquely identified by the combination of `deployment_namespace + blueprint_name + hs_name`. For example, if you were running `BlueprintAlice` then a unique string might look like `5_alice_hs1` where `5` is an atomic, monotonically increasing integer incremented when `Deploy()` is called (see #113 for more info). In a parallel by default world this is no longer true because the `deployment_namespace` is not shared between different test processes. This means we cannot co-ordinate non-clashing namespaces like before. Instead, we will bring in another namespace for the test process (which in #119 will be on a per-package basis, hence the name `PackageNamespace`). As of this PR, literally everything Complement makes (images, containers, networks, etc) are prefixed with this package namespace, which allows multiple complement instances to share the same underlying docker daemon, with caveats: - Creating CA certificates will race and needs a lockfile to prevent 2 processes trying to create the certificate at the same time. - Complement federation servers cannot run together due to trying to bind to `:8448` at the same time. That being said, this PR should enable the parallelisation of a number of CS API only tests, which will come in another PR. * Set to pkg to keep the ref format of committed containers valid
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's always been a weird quirk that you need to specify a unique namespace string when
deploying containers. This is for parallel tests such that if 2 tests deploy
BlueprintAlice they get unique container names.
This PR changes the API so that we just increment an atomic
counter and use that as the namespace.