Repository navigation
K8s plugin: Support custom CNI manifest - #2254
Conversation
Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
6797cbb to
b2c8fd3
Compare
Code Coverage
|
|
b2c8fd3 to
10754bf
Compare
egernst
left a comment
There was a problem hiding this comment.
Cool -- looks good Kathryn. Thanks for the updated docs/example
| + "\(kubeconfigEnv) kubectl apply -f /tmp/kindnet.yaml" | ||
| log.info("Applying CNI manifest", metadata: ["node": "\(nodeID)"]) | ||
| let manifest = try await loadCNIManifest(path: cniManifestPath, log: log) | ||
| let apply = "\(kubeconfigEnv) kubectl apply -f - <<'EOF'\n\(manifest)\nEOF" |
There was a problem hiding this comment.
Can things get weird here if the user submits a yaml that "accidentally" includes EOF? Should we feed it a real stdin, or base64-encoding the payload, or?
There was a problem hiding this comment.
Also, I think a particularly large manifest may hit E2BIG - I think this would also be solved if we considered a pipe?
There was a problem hiding this comment.
I don't know what the right answer for this is from a UX perspective. I'd like to investigate that in a separate PR if that's okay with you.
There was a problem hiding this comment.
Yeah, I think the UX would be the same, just a matter of how we handle it. I'm fine doing this as a follow up. #2273
|
|
||
| static func loadCNIManifest(path: String?, log: Logger) async throws -> String { | ||
| if let path { | ||
| guard let contents = try? String(contentsOfFile: path, encoding: .utf8) else { |
There was a problem hiding this comment.
should we do/catch so we can print out the actual error (could be a read error as well?)
| } | ||
| } | ||
|
|
||
| @Test func customPathTakesPrecedenceOverBundledCNI() async throws { |
There was a problem hiding this comment.
I think this is effectively covering the same as customPathReturnsItsContents().
A local friend of mine had this suggestion:
Line 76 already asserted result == contents, and contents is the literal "custom-cni-marker". So by the time you reach line 77, result is known to be that string, which self-evidently doesn't contain "kindnet". The assertion is true as a consequence of the previous line — it has no independent power to catch anything.
Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
10754bf to
1e72a84
Compare
Type of Change
Motivation and Context
This PR adds a new flag to the k8s create command to allow users to specify an optional path to a CNI manifest that they wish to use for the cluster instead of the bundled kindnet CNI manifest.
I tested this with cilium by doing the following:
Testing