Skip to content

Enable dynamic changes in the router for non-kube environments #2356

Merged
nluaces merged 37 commits intoskupperproject:mainfrom
nluaces:2337-system-adaptor-process
Apr 10, 2026
Merged

Enable dynamic changes in the router for non-kube environments #2356
nluaces merged 37 commits intoskupperproject:mainfrom
nluaces:2337-system-adaptor-process

Conversation

@nluaces
Copy link
Copy Markdown
Member

@nluaces nluaces commented Jan 18, 2026

Implements #2337
Branch based on #2334, the proper commits that implement this issue are bb8ab35 and the following.

flowchart TD
   id1([new listener resource is created])-->|triggers|InputResourceHandler-->Q{site exists?}
   Q -->|Yes| A[Refresh Router Config]
   Q -->|No| B[Bootstrap]
Loading
flowchart LR
  SystemAdaptorHandler-->|has a callback|RouterStatusHandler
  SystemAdaptorHandler-->|spawns|processRouterConfig-->id2([reconciliates the router config file with the router])
Loading

The InputResourceHandler was modified to not bootstrap in case a site is already created in the namespace.

EDIT: A new flag called --reload-type is available in the skupper system install command. It has priority over the env. variable.

@nluaces nluaces self-assigned this Jan 18, 2026
@nluaces nluaces changed the title Enable dynamic changes in the router for non-kube environments (wip) Enable dynamic changes in the router for non-kube environments Feb 1, 2026
@nluaces nluaces marked this pull request as ready for review February 1, 2026 17:31
Copy link
Copy Markdown
Member

@fgiorgetti fgiorgetti left a comment

Choose a reason for hiding this comment

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

My impression so far is that it is on the right track.
Really nice work @nluaces!

A few other comments I have so far:

  • The site name in the router config is changing (we need to check if this will cause issues to the console or vanflow metrics)
  • As I already added, the system controller update and the update of the router container for the controlled namespaces is something we need to figure out still

@nluaces nluaces force-pushed the 2337-system-adaptor-process branch from 0eb07fd to d4bc626 Compare February 10, 2026 10:22
@nluaces
Copy link
Copy Markdown
Member Author

nluaces commented Feb 10, 2026

  • The site name in the router config is changing (we need to check if this will cause issues to the console or vanflow metrics)

I checked that the name of the site was not changing when the controller just refresh (for example, when a listener has been created); do you have any examples for me to understand this better?

@nluaces nluaces requested a review from fgiorgetti February 10, 2026 16:02
@nluaces nluaces linked an issue Feb 12, 2026 that may be closed by this pull request
@nluaces nluaces force-pushed the 2337-system-adaptor-process branch from 9c2e556 to d63b383 Compare February 13, 2026 17:16
@nluaces nluaces requested a review from fgiorgetti February 23, 2026 15:25
… given that it is going to be set up by the CLI with the install command. remove system adaptor callback in case the reload type is manual
@nluaces nluaces requested a review from fgiorgetti March 9, 2026 09:45
Copy link
Copy Markdown
Member

@fgiorgetti fgiorgetti left a comment

Choose a reason for hiding this comment

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

The system stop command should use a similar validation done to the system reload command, which performs the following check:

        if nsPlatform != currentPlatform {
            return nil, fmt.Errorf("existing namespace uses %q platform and it cannot change to %q", nsPlatform, currentPlatform)
        }

At present if someone runs system stop --platform podman but the actual platform is docker, the container is being left behind.

@nluaces nluaces requested a review from fgiorgetti March 27, 2026 16:25
@nluaces nluaces requested a review from fgiorgetti April 6, 2026 15:56
Copy link
Copy Markdown
Member

@fgiorgetti fgiorgetti left a comment

Choose a reason for hiding this comment

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

Looks great now! Thank you very much, @nluaces !
Tested against a matrix of:

  • Platforms: podman, docker
  • Users: root, regular-user
  • Reload type: auto, manual
  • Method: CLI, Ansible (skupper.v2 collection)

@nluaces nluaces merged commit 725a609 into skupperproject:main Apr 10, 2026
2 checks passed
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.

Create system-adaptor that process dynamic changes in the router

2 participants