Skip to content

hostrouting: add ConfigureStaticRoutes and CheckRouteOverlap as phases.Task#159

Draft
Copilot wants to merge 7 commits intomainfrom
copilot/reimplement-pr-151-new-task-interface
Draft

hostrouting: add ConfigureStaticRoutes and CheckRouteOverlap as phases.Task#159
Copilot wants to merge 7 commits intomainfrom
copilot/reimplement-pr-151-new-task-interface

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026 •
edited
Loading

  • Add HostRoutingConfig struct to pkg/config/structs.go
  • Create pkg/hostrouting/ package (renamed from pkg/routing/)
    • assets/ — service and script template files using Before=systemd-nspawn@.service
    • configure_static_routes.go + tests
    • check_route_overlap.go + tests
    • hostrouting.go — Configure(cfg, logger) phases.Task combining both tasks via phases.Serial
  • Wire single hostrouting.Configure(b.cfg, b.logger) call into pkg/bootstrapper/bootstrapper.go
  • Run gofmt on all changed files (fixed alignment in pkg/config/structs.go)
  • Merged latest main (resolved go.sum conflict for unbounded v0.1.2 → v0.1.3)
  • Build and tests pass

Copilot AI linked an issue Apr 29, 2026 that may be closed by this pull request
reimplement PR 151 with new task interface #158
Open
Copilot AI changed the title [WIP] Reimplement PR 151 with new task interface routing: add ConfigureStaticRoutes and CheckRouteOverlap as phases.Task Apr 29, 2026
Copilot finished work on behalf of bcho April 29, 2026 19:54
Copilot AI requested a review from bcho April 29, 2026 19:54
bcho
bcho requested changes Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@bcho bcho left a comment

Choose a reason for hiding this comment

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

@copilot

Comment thread pkg/routing/assets/static-routes.service Outdated
# Route install must happen before kubelet tries to reach cluster services
# whose CIDR may otherwise be shadowed by provider-installed connected
# routes (e.g. the Azure InfiniBand fabric /16 on ND-isr SKUs).
Before=kubelet.service
Copy link
Copy Markdown
Member

@bcho bcho Apr 29, 2026

Choose a reason for hiding this comment

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

there is no kubelet.service on the host in current impl. We should apply this before nspawn machine service instead.

Copy link
Copy Markdown
Contributor Author

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

Fixed in 1b49cea. Both service files now use Before=systemd-nspawn@.service and RequiredBy=systemd-nspawn@.service (matching the pattern used by nftables-flush.service in the unbounded library), since kubelet.service lives inside the nspawn container, not on the host.

Comment thread pkg/config/structs.go Outdated
Node NodeConfig `json:"node"`
Paths PathsConfig `json:"paths"`
Npd NPDConfig `json:"npd"`
Routing RoutingConfig `json:"routing"`
Copy link
Copy Markdown
Member

@bcho bcho Apr 29, 2026

Choose a reason for hiding this comment

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

@copilot rename this to HostRouting and HostRoutingConfig to avoid confusion

Copy link
Copy Markdown
Contributor Author

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

Done in 1b49cea — renamed RoutingConfig → HostRoutingConfig and the Config field Routing → HostRouting.

@bcho
Copy link
Copy Markdown
Member

bcho commented Apr 29, 2026

@chokevin do we have ways to test this feature without using real GPU sku?

@bcho
address review feedback: use systemd-nspawn@.service and rename Routi…
1b49cea 
…ngConfig to HostRoutingConfig

Agent-Logs-Url: https://github.com/Azure/AKSFlexNode/sessions/4c849c1e-b508-489a-b4f1-48bcc7921340

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>
Copilot finished work on behalf of bcho April 29, 2026 20:13
Copilot AI requested a review from bcho April 29, 2026 20:13
bcho
bcho requested changes Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@bcho bcho left a comment

Choose a reason for hiding this comment

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

@copilot

Comment thread pkg/bootstrapper/bootstrapper.go Outdated
Copilot AI changed the title routing: add ConfigureStaticRoutes and CheckRouteOverlap as phases.Task hostrouting: add ConfigureStaticRoutes and CheckRouteOverlap as phases.Task Apr 29, 2026
Copilot finished work on behalf of bcho April 29, 2026 20:23
Copilot AI requested a review from bcho April 29, 2026 20:23
bcho
bcho reviewed Apr 29, 2026
View reviewed changes
Comment thread pkg/hostrouting/hostrouting.go Outdated
bcho
bcho requested changes Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@bcho bcho left a comment

Choose a reason for hiding this comment

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

@copilot

Copilot finished work on behalf of bcho April 29, 2026 20:28
Copilot AI requested a review from bcho April 29, 2026 20:28
github-advanced-security[bot]
github-advanced-security AI found potential problems Apr 29, 2026
View reviewed changes
Comment thread pkg/hostrouting/configure_static_routes.go

// runCmd runs the given command and logs its output via logger.
func runCmd(ctx context.Context, logger *slog.Logger, name string, args ...string) error {
cmd := exec.CommandContext(ctx, name, args...) //nolint:gosec // name is a constant binary path
bcho
bcho requested changes Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@bcho bcho left a comment

Choose a reason for hiding this comment

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

@copilot run go format on code

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 29, 2026

@copilot run go format on code

Done in 87c3ecb — ran gofmt -w across all changed files; the only diff was realigning the struct field columns in pkg/config/structs.go after the HostRouting field was added.

Copilot finished work on behalf of bcho April 29, 2026 22:09
Copilot AI requested a review from bcho April 29, 2026 22:09
@bcho
Copy link
Copy Markdown
Member

bcho commented Apr 30, 2026

@copilot rebase

@bcho
Merge remote-tracking branch 'origin/main' into copilot/reimplement-p…
f4b0bd4 
…r-151-new-task-interface

Co-authored-by: bcho <1975118+bcho@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 30, 2026

@copilot rebase

Done in f4b0bd4 — merged origin/main into the branch (resolving a go.sum conflict for unbounded v0.1.2 → v0.1.3). Build and tests pass.

Copilot finished work on behalf of bcho April 30, 2026 23:13
Copilot AI deployed to e2e-testing April 30, 2026 23:17 Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bcho bcho Awaiting requested review from bcho

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

1 more reviewer

@github-advanced-security github-advanced-security[bot] github-advanced-security[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@bcho bcho

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

reimplement PR 151 with new task interface

3 participants

@bcho @github-advanced-security