Skip to content

Conversation

@nluaces
Copy link
Member

@nluaces nluaces commented Dec 2, 2025

Implements #2333

Changes:

  • The namespace controller has an additional watcher for custom resources created in the designated directories.
  • This feature is enabled by default ("auto"). To disable it, export the environment variable SKUPPER_SYSTEM_RELOAD_TYPE to "manual"
  • CLI change: the command skupper system install needs to pass said env. variable to the system-controller pod.

Additional information:

  • These changes only apply to podman and docker platforms and they can be tested by running:
export SKUPPER_SYSTEM_CONTROLLER_IMAGE=quay.io/nluaces/system-controller:latest

@nluaces nluaces self-assigned this Dec 2, 2025
@nluaces nluaces force-pushed the 2333-system-controller-input-watcher branch from 163082a to 853219d Compare December 2, 2025 17:24
Copy link
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.

It would be nice to include a message in the startup banner of the system controller, displaying the status of the autoreload definition.

Another thing I am missing here is the fact that if you delete a Site and/or all resources from a given namespace, the Teardown is not being executed.

Copy link
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.

It is working great now.

One thing that is still missing is a log message indicating whether reload type is auto or manual as part of the startup banner.

endpoint = "unix:///var/run/podman.sock"
if s.Platform == "docker" {
endpoint = "unix:///run/docker.sock"
endpoint = "unix:///var/run/docker.sock"
Copy link
Member

Choose a reason for hiding this comment

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

I believe that the old value is still appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have double checked this using docker as platform and leaving unix:///run/docker.sock:

2026/02/09 16:15:26 ERROR Failed to bootstrap: failed to render site state: failed to create container client: Container engine is not available on provided endpoint - stat /run/docker.sock: no such file or directory component=input.resource.handler namespace=default

I think the problem is similar to podman platform, regarding where the container endpoint is mapped to the socket.

Copy link
Member

Choose a reason for hiding this comment

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

I believe that /var/run should work, as it is supposed to be just a lihk to /run, but I am wondering when did it change. Which version of docker do you have, Noe?
Anyway, if we want to change this, then there are several other locations in the repo that must be updated as well. If something has changed recently, then better changing it everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Docker version 29.2.1, build a5c7197

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway, if we want to change this, then there are several other locations in the repo that must be updated as well. If something has changed recently, then better changing it everywhere.

I agree. Should be changed everywhere as part of this pull request? I can also do it as part of another issue.

Copy link
Member

Choose a reason for hiding this comment

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

You can do it here, at least all references will be using /var/run/docker.sock.

Copy link
Member Author

Choose a reason for hiding this comment

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

@fgiorgetti let me know if these commits work for you:
867d6ac
a90b119

I haven't done these changes in the implementations of install/uninstall commands, as they are not meant to be executed inside the container (same goes for bootstrap.sh and system_controller.sh).

@nluaces nluaces force-pushed the 2333-system-controller-input-watcher branch from dfeb6a6 to dab70d7 Compare February 9, 2026 14:09
@nluaces
Copy link
Member Author

nluaces commented Feb 9, 2026

One thing that is still missing is a log message indicating whether reload type is auto or manual as part of the startup banner.

sorry! fixed now.

@nluaces nluaces requested a review from fgiorgetti February 9, 2026 17:57
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.

Reload automatically changes in custom resources for docker and podman system platforms

2 participants