Skip to content

refactor: extract K8s logic into ContainerK8s library target - #2079

Open
jshi991 wants to merge 3 commits into
apple:mainfrom
jshi991:k8s-plugin-refactor
Open

refactor: extract K8s logic into ContainerK8s library target#2079
jshi991 wants to merge 3 commits into
apple:mainfrom
jshi991:k8s-plugin-refactor

Conversation

@jshi991

@jshi991 jshi991 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Closes #2078
Move all K8s sources from the container-k8s executable into a new ContainerK8s library target. Sources/Plugins/K8s/ becomes a thin entry point (K8sMain.swift) that calls K8sCommand.main().

Also make iptables MSS clamping non-fatal: if xt_TCPMSS is unavailable in the VM kernel the node prep no longer aborts; a warning is logged instead.

@jshi991
jshi991 force-pushed the k8s-plugin-refactor branch from 60bd997 to 6c30a15 Compare August 6, 2026 18:44

@jglogan jglogan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few changes, see comments. Thanks @jshi991!

Comment thread Sources/ContainerTestSupport/WarmupImage.swift
Comment thread Sources/Plugins/K8s/K8sCommand.swift
Comment thread Sources/ContainerK8s/K8sHelper.swift Outdated
ctr -n k8s.io images tag registry.k8s.io/pause:3.10 registry.k8s.io/pause:3.10.1 2>/dev/null || true
iptables -t mangle -A OUTPUT -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --set-mss 1220
iptables -t mangle -A FORWARD -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --set-mss 1220
/usr/sbin/iptables-nft -t mangle -A OUTPUT -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --set-mss 1220 2>/dev/null || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why dump the diagnostic output to /dev/null? I think we'd want to see that.

Why don't we let the command just fail if it doesn't work, instead of ignoring failure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup ill fail and log

Comment thread Sources/ContainerK8s/K8sHelper.swift Outdated
ctr -n k8s.io images tag registry.k8s.io/pause:3.10 registry.k8s.io/pause:3.10.1 2>/dev/null || true
iptables -t mangle -A OUTPUT -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --set-mss 1220
iptables -t mangle -A FORWARD -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --set-mss 1220
/usr/sbin/iptables-nft -t mangle -A OUTPUT -p tcp --tcp-flags SYN,RST SYN -j TCPMSS --set-mss 1220 2>/dev/null || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is different than the rest of the commit, you mind just adding this one as its own commit and describe the rationale on it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup I can do that do you want this to be left with just the two commits

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jshi991 Yes. Ideally three commits:

@jshi991
jshi991 force-pushed the k8s-plugin-refactor branch from b2486e0 to ae55815 Compare August 7, 2026 02:17
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Code Coverage

Tier Line Coverage
Unit 24.27%
Integration 66.78%
Combined 76.14%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[k8s] Extract K8s commands into a ContainerK8s library target

3 participants