Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/kube/certificates/mgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ func secretWithOwnerRef(secret *corev1.Secret, ref metav1.OwnerReference) *corev
return secret
}

// managedWithOwnerHosts sets up a Certificiate with skupper controlled and owner hosts annotations
// managedWithOwnerHosts sets up a Certificate with skupper controlled and owner hosts annotations
func managedWithOwnerHosts(t *testing.T, cert *skupperv2alpha1.Certificate, ref metav1.OwnerReference, hosts ...string) *skupperv2alpha1.Certificate {
t.Helper()
cert.ObjectMeta.OwnerReferences = append(cert.ObjectMeta.OwnerReferences, ref)
Expand Down
55 changes: 37 additions & 18 deletions internal/nonkube/bootstrap/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,41 @@ func Install(platform string, reloadType string) error {
return err
}

if reloadType == "" {
reloadType = utils.DefaultStr(os.Getenv(types.ENV_SYSTEM_AUTO_RELOAD),
types.SystemReloadTypeManual)
}

config, err := configEnvVariables(platform)
if err != nil {
return err
}

containerName := fmt.Sprintf("%s-skupper-controller", config.username)

isContainerAlreadyRunningInPodman := IsContainerRunning(containerName, types.PlatformPodman)
foundInPodman, podmanState := FindContainer(containerName, types.PlatformPodman)

if isContainerAlreadyRunningInPodman {
fmt.Printf("Warning: The system controller container %q is already running in Podman.\n", containerName)
if foundInPodman {
fmt.Printf("Warning: The system controller container %q is already present in Podman (state: %s).\n", containerName, podmanState)
if reloadType == types.SystemReloadTypeAuto {
enabler := newSiteServiceEnablerInstaller()
if err = enabler.Install(); err != nil {
return fmt.Errorf("failed to install skupper site service enabler: %v", err)
}
}
return nil
}

isContainerAlreadyRunningInDocker := IsContainerRunning(containerName, types.PlatformDocker)
foundInDocker, dockerState := FindContainer(containerName, types.PlatformDocker)

if isContainerAlreadyRunningInDocker {
fmt.Printf("Warning: The system controller container %q is already running in Docker.\n", containerName)
if foundInDocker {
fmt.Printf("Warning: The system controller container %q is already present in Docker (state: %s).\n", containerName, dockerState)
if reloadType == types.SystemReloadTypeAuto {
enabler := newSiteServiceEnablerInstaller()
if err = enabler.Install(); err != nil {
return fmt.Errorf("failed to install skupper site service enabler: %v", err)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return nil
}

Expand All @@ -72,11 +89,6 @@ func Install(platform string, reloadType string) error {
}
fmt.Printf("Pulled system-controller image: %s\n", images.GetSystemControllerImageName())

if reloadType == "" {
reloadType = utils.DefaultStr(os.Getenv(types.ENV_SYSTEM_AUTO_RELOAD),
types.SystemReloadTypeManual)
}

env := map[string]string{
"CONTAINER_ENDPOINT": config.containerEndpoint,
"SKUPPER_OUTPUT_PATH": config.hostDataHome,
Expand Down Expand Up @@ -144,6 +156,13 @@ func Install(platform string, reloadType string) error {
return fmt.Errorf("failed to create system-controller systemd service: %v", err)
}

if reloadType == types.SystemReloadTypeAuto {
enabler := newSiteServiceEnablerInstaller()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Safer to let it just run when not running in a container.
If someone, for example, installs the controller using Ansible (which runs the cli image in a container), then it should not be installed.

if err = enabler.Install(); err != nil {
return fmt.Errorf("failed to install skupper site service enabler: %v", err)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

fmt.Printf("Platform %s is now configured for Skupper\n", platform)

return nil
Expand Down Expand Up @@ -262,7 +281,7 @@ func createSystemdService(container container.Container, platform string) error
return nil
}

func IsContainerRunning(containerName string, platform types.Platform) bool {
func FindContainer(containerName string, platform types.Platform) (bool, string) {

endpoint := fmt.Sprintf("unix://%s/podman/podman.sock", api.GetRuntimeDir())
if platform == types.PlatformDocker {
Expand All @@ -271,19 +290,19 @@ func IsContainerRunning(containerName string, platform types.Platform) bool {

cli, err := internalclient.NewCompatClient(endpoint, "")
if err != nil {
return false
return false, ""
}

containers, err := cli.ContainerList()
if err != nil {
return false
return false, ""
}

for _, container := range containers {
if container.Name == containerName {
return true
for _, c := range containers {
if c.Name == containerName {
return true, c.State
}
}

return false
return false, ""
}
168 changes: 168 additions & 0 deletions internal/nonkube/bootstrap/site_service_enabler_installer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
package bootstrap

import (
"bytes"
_ "embed"
"fmt"
"os"
"os/exec"
"path"
"text/template"

"github.com/skupperproject/skupper/pkg/nonkube/api"
)

//go:embed site_service_enabler_service.template
var siteServiceEnablerServiceTemplate string

//go:embed site_service_enabler_script.template
var siteServiceEnablerScriptTemplate string

const (
siteServiceEnablerRootSystemdBasePath = "/etc/systemd/system"
siteServiceEnablerName = "skupper-site-service-enabler"
siteServiceEnablerServiceFile = siteServiceEnablerName + ".service"
siteServiceEnablerWrapperScript = siteServiceEnablerName
)

type siteServiceEnablerData struct {
ScriptPath string
WantedBy string
}

type siteServiceEnablerScriptData struct {
NamespacesDir string
SystemdUnitDir string
SystemctlArgs string
}

type SiteServiceEnablerInstaller struct {
uid int
rootSystemdBasePath string
scriptDir string
command func(string, ...string) *exec.Cmd
}

func newSiteServiceEnablerInstaller() *SiteServiceEnablerInstaller {
return &SiteServiceEnablerInstaller{
uid: os.Getuid(),
rootSystemdBasePath: siteServiceEnablerRootSystemdBasePath,
scriptDir: path.Join(api.GetSystemControllerPath(), "bin"),
command: exec.Command,
}
}

func (s *SiteServiceEnablerInstaller) Install() error {
if s.isRunning() {
return nil
}
if err := os.MkdirAll(s.scriptDir, 0755); err != nil {
return fmt.Errorf("unable to create script directory %q: %w", s.scriptDir, err)
}
if err := os.MkdirAll(s.userSystemdDir(), 0755); err != nil {
return fmt.Errorf("unable to create systemd unit directory %q: %w", s.userSystemdDir(), err)
}

scriptPath := path.Join(s.scriptDir, siteServiceEnablerWrapperScript)
if err := s.renderFile(siteServiceEnablerScriptTemplate, s.scriptData(), scriptPath, 0755); err != nil {
return fmt.Errorf("unable to write wrapper script %q: %w", scriptPath, err)
}

serviceFile := s.unitPath(siteServiceEnablerServiceFile)
if err := s.renderFile(siteServiceEnablerServiceTemplate, s.templateData(scriptPath), serviceFile, 0644); err != nil {
return fmt.Errorf("unable to write site enabler service unit: %w", err)
}

if err := s.systemctl("daemon-reload"); err != nil {
return fmt.Errorf("daemon-reload failed: %w", err)
}
if err := s.systemctl("enable", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("unable to enable %s: %w", siteServiceEnablerServiceFile, err)
}
if err := s.systemctl("start", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("unable to start %s: %w", siteServiceEnablerServiceFile, err)
}

return nil
}

func (s *SiteServiceEnablerInstaller) Remove() error {
unitFile := s.unitPath(siteServiceEnablerServiceFile)
if _, err := os.Stat(unitFile); err == nil {
if err := s.systemctl("stop", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("failed to stop %s: %w", siteServiceEnablerServiceFile, err)
}
if err := s.systemctl("disable", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("failed to disable %s: %w", siteServiceEnablerServiceFile, err)
}
if err := os.Remove(unitFile); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove unit file: %w", err)
}
}
Comment on lines +91 to +101

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Return errors that prevent inspection of the unit file.

At Line 91, a permission or I/O error from os.Stat is handled as if the unit were absent. Remove can then return success after removing the wrapper and reloading systemd while the helper unit remains installed. Return errors other than os.IsNotExist(err).

Proposed fix
-	if _, err := os.Stat(unitFile); err == nil {
+	if _, err := os.Stat(unitFile); err == nil {
 		if err := s.systemctl("stop", siteServiceEnablerServiceFile); err != nil {
 			return fmt.Errorf("failed to stop %s: %w", siteServiceEnablerServiceFile, err)
 		}
 		if err := s.systemctl("disable", siteServiceEnablerServiceFile); err != nil {
 			return fmt.Errorf("failed to disable %s: %w", siteServiceEnablerServiceFile, err)
 		}
 		if err := os.Remove(unitFile); err != nil && !os.IsNotExist(err) {
 			return fmt.Errorf("failed to remove unit file: %w", err)
 		}
+	} else if !os.IsNotExist(err) {
+		return fmt.Errorf("failed to inspect unit file: %w", err)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if _, err := os.Stat(unitFile); err == nil {
if err := s.systemctl("stop", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("failed to stop %s: %w", siteServiceEnablerServiceFile, err)
}
if err := s.systemctl("disable", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("failed to disable %s: %w", siteServiceEnablerServiceFile, err)
}
if err := os.Remove(unitFile); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove unit file: %w", err)
}
}
if _, err := os.Stat(unitFile); err == nil {
if err := s.systemctl("stop", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("failed to stop %s: %w", siteServiceEnablerServiceFile, err)
}
if err := s.systemctl("disable", siteServiceEnablerServiceFile); err != nil {
return fmt.Errorf("failed to disable %s: %w", siteServiceEnablerServiceFile, err)
}
if err := os.Remove(unitFile); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove unit file: %w", err)
}
} else if !os.IsNotExist(err) {
return fmt.Errorf("failed to inspect unit file: %w", err)
}

if err := os.Remove(path.Join(s.scriptDir, siteServiceEnablerWrapperScript)); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove wrapper script: %w", err)
}
if err := s.systemctl("daemon-reload"); err != nil {
return fmt.Errorf("daemon-reload failed after remove: %w", err)
}
return nil
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

func (s *SiteServiceEnablerInstaller) scriptData() siteServiceEnablerScriptData {
systemctlArgs := ""
if s.uid != 0 {
systemctlArgs = "--user "
}
return siteServiceEnablerScriptData{
NamespacesDir: api.GetDefaultOutputNamespacesPath(),
SystemdUnitDir: s.userSystemdDir(),
SystemctlArgs: systemctlArgs,
}
}

func (s *SiteServiceEnablerInstaller) templateData(scriptPath string) siteServiceEnablerData {
wantedBy := "default.target"
if s.uid == 0 {
wantedBy = "multi-user.target"
}
return siteServiceEnablerData{
ScriptPath: scriptPath,
WantedBy: wantedBy,
}
}

func (s *SiteServiceEnablerInstaller) unitPath(unit string) string {
return path.Join(s.userSystemdDir(), unit)
}

func (s *SiteServiceEnablerInstaller) userSystemdDir() string {
if s.uid == 0 {
return s.rootSystemdBasePath
}
return path.Join(api.GetConfigHome(), "systemd", "user")
}

func (s *SiteServiceEnablerInstaller) isRunning() bool {
return s.systemctl("is-active", "--quiet", siteServiceEnablerServiceFile) == nil
}

func (s *SiteServiceEnablerInstaller) systemctl(args ...string) error {
var fullArgs []string
if s.uid != 0 {
fullArgs = append(fullArgs, "--user")
}
fullArgs = append(fullArgs, args...)
return s.command("systemctl", fullArgs...).Run()
}

func (s *SiteServiceEnablerInstaller) renderFile(tmplText string, data any, dst string, mode os.FileMode) error {
tmpl, err := template.New("").Parse(tmplText)
if err != nil {
return err
}
var buf bytes.Buffer
if err = tmpl.Execute(&buf, data); err != nil {
return err
}
return os.WriteFile(dst, buf.Bytes(), mode)
}
Loading
Loading