Add --labels for networks.
#600
Merged
Conversation
This file contains hidden or 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
- Closes apple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.
Comment on lines
+47
to
+52
| let container = try decoder.container(keyedBy: CodingKeys.self) | ||
|
|
||
| id = try container.decode(String.self, forKey: .id) | ||
| mode = try container.decode(NetworkMode.self, forKey: .mode) | ||
| subnet = try container.decodeIfPresent(String.self, forKey: .subnet) | ||
| labels = try container.decodeIfPresent([String: String].self, forKey: .labels) ?? [:] |
Contributor
There was a problem hiding this comment.
Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?
| // using our configuration data, as the one from the helper doesn't include | ||
| // metadata. | ||
| guard case .running(_, let status) = try await client.state() else { | ||
| throw ContainerizationError(.exists, message: "network \(configuration.id) failed to start") |
Contributor
There was a problem hiding this comment.
would .invalidState be better?
Contributor
Author
There was a problem hiding this comment.
yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it
| func run() async throws { | ||
| let config = NetworkConfiguration(id: self.name, mode: .nat) | ||
| let parsedLabels = Utility.parseKeyValuePairs(labels) | ||
| let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels) |
Contributor
There was a problem hiding this comment.
should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?
Contributor
Author
There was a problem hiding this comment.
yes, good call, I decided to make init() for NetworkConfiguration validate and throw
realrajaryan
approved these changes
Sep 15, 2025
Mcrich23
added a commit
to Mcrich23/container
that referenced
this pull request
Sep 17, 2025
commit 449f1d2 Author: J Logan <[email protected]> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closes apple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <[email protected]> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixes apple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <[email protected]> commit 79cc363 Author: J Logan <[email protected]> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closes apple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <[email protected]> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closes apple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
container network create --labeloption. #557..upToNextOptionfor labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.Type of Change
Motivation and Context
Consistent features and UX across managed resources.
Testing