Skip to content

Commit

Permalink
Fixes displaying of podman sites recommandation (#1321)
Browse files Browse the repository at this point in the history
  • Loading branch information
fgiorgetti authored Dec 15, 2023
1 parent 223de44 commit 628277f
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 43 deletions.
2 changes: 2 additions & 0 deletions client/podman/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ type PodmanRestClient struct {
endpoint string
}

type RestClientFactory func(endpoint, basePath string) (*PodmanRestClient, error)

func NewPodmanClient(endpoint, basePath string) (*PodmanRestClient, error) {
var err error

Expand Down
93 changes: 66 additions & 27 deletions cmd/skupper/skupper_podman.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"fmt"
"io"
"os"

"github.com/skupperproject/skupper/api/types"
Expand All @@ -19,14 +20,20 @@ var SkupperPodmanCommands = []string{
}

type SkupperPodman struct {
cli *clientpodman.PodmanRestClient
currentSite *podman.Site
site *SkupperPodmanSite
token *SkupperPodmanToken
link *SkupperPodmanLink
service *SkupperPodmanService
cliFactory clientpodman.RestClientFactory
cli *clientpodman.PodmanRestClient
currentSite *podman.Site
siteHandlerFactory podman.SiteHandlerFactory
site *SkupperPodmanSite
token *SkupperPodmanToken
link *SkupperPodmanLink
service *SkupperPodmanService
exit exitHandler
output io.Writer
}

type exitHandler func(code int)

func (s *SkupperPodman) Site() SkupperSiteClient {
if s.site != nil {
return s.site
Expand Down Expand Up @@ -85,6 +92,10 @@ func (s *SkupperPodman) NewClient(cmd *cobra.Command, args []string) {
var endpoint string
var isInitCmd bool
exitOnError := true
if s.output == nil {
s.output = os.Stdout
}
out := s.output
switch cmd.Name() {
case "init":
// require site not present
Expand All @@ -97,52 +108,80 @@ func (s *SkupperPodman) NewClient(cmd *cobra.Command, args []string) {
default:
podmanCfg, err := podman.NewPodmanConfigFileHandler().GetConfig()
if err != nil {
fmt.Println("error reading current podman endpoint")
fmt.Fprintln(out, "error reading current podman endpoint")
return
}
endpoint = podmanCfg.Endpoint
}

c, err := clientpodman.NewPodmanClient(endpoint, "")
if s.cliFactory == nil {
s.cliFactory = clientpodman.NewPodmanClient
}
if s.exit == nil {
s.exit = os.Exit
}
c, err := s.cliFactory(endpoint, "")
if err != nil {
if exitOnError {
fmt.Printf("Podman endpoint is not available: %s",
fmt.Fprintf(out, "Podman endpoint is not available: %s",
utils.DefaultStr(endpoint, clientpodman.GetDefaultPodmanEndpoint()))
fmt.Println()
os.Exit(1)
fmt.Fprintln(out)
recommendation := `
Recommendation:
Make sure you have an active podman endpoint available.
On most systems you can execute:
systemctl --user enable --now podman.socket
Alternatively you could also create your own service that runs:
podman system service --time=0 <URI>
You can get concrete examples through:
podman help system service`
fmt.Fprintln(out, recommendation)
s.exit(1)
}
return
}
// only if default endpoint is available or correct endpoint is set
s.cli = c

// Ensure that site does not exist on init, but exists for all other commands
siteHandler, err := podman.NewSitePodmanHandler(endpoint)
if s.siteHandlerFactory == nil {
s.siteHandlerFactory = podman.NewSiteHandler
}
siteHandler, err := s.siteHandlerFactory(endpoint)
if err != nil {
fmt.Printf("error verifying existing skupper site - %s", err)
fmt.Println()
os.Exit(1)
fmt.Fprintf(out, "error verifying existing skupper site - %s", err)
fmt.Fprintln(out)
s.exit(1)
return
}
currentSite, err := siteHandler.Get()
if isInitCmd {
// Validating if site is already initialized
if err == nil && currentSite != nil {
fmt.Printf("Skupper has already been initialized for user '" + podman.Username + "'.")
fmt.Println()
os.Exit(0)
fmt.Fprintf(out, "Skupper has already been initialized for user '"+podman.Username+"'.")
fmt.Fprintln(out)
s.exit(0)
return
}
} else if !utils.StringSliceContains([]string{"version", "delete"}, cmd.Name()) {
// All other commands, but version and delete, must stop now
if err != nil {
fmt.Printf("Skupper is not enabled for user '%s'", podman.Username)
fmt.Println()
if siteHandler.AnyResourceLeft() {
fmt.Println("Reason:", err)
fmt.Println()
fmt.Println("There are podman resources missing or left from an earlier installation")
fmt.Println("To clean them up, run: skupper delete")
fmt.Fprintf(out, "Skupper is not enabled for user '%s'", podman.Username)
fmt.Fprintln(out)
siteHandlerPodman, ok := siteHandler.(*podman.SiteHandler)
if ok && siteHandlerPodman.AnyResourceLeft() {
fmt.Fprintln(out, "Reason:", err)
fmt.Fprintln(out)
fmt.Fprintln(out, "There are podman resources missing or left from an earlier installation")
fmt.Fprintln(out, "To clean them up, run: skupper delete")
}
os.Exit(0)
s.exit(0)
return
}
}
if currentSite != nil {
Expand Down
16 changes: 0 additions & 16 deletions cmd/skupper/skupper_podman_site.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,26 +78,10 @@ func (s *SkupperPodmanSite) Create(cmd *cobra.Command, args []string) error {
siteHandler, err := podman.NewSitePodmanHandler(site.PodmanEndpoint)
if err != nil {
initErr := fmt.Errorf("Unable to initialize Skupper - %w", err)
recommendation := `
Recommendation:
Make sure you have an active podman endpoint available.
On most systems you can execute:
systemctl --user enable --now podman.socket
Alternatively you could also create your own service that runs:
podman system service --time=0 <URI>
You can get concrete examples through:
podman help system service`

cmd.SilenceUsage = true
cmd.SilenceErrors = true
fmt.Println("Error:", initErr)
fmt.Println(recommendation)
return initErr
}

Expand Down
177 changes: 177 additions & 0 deletions cmd/skupper/skupper_podman_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
package main

import (
"bytes"
"fmt"
"strings"
"testing"

clientpodman "github.com/skupperproject/skupper/client/podman"
"github.com/skupperproject/skupper/pkg/domain"
"github.com/skupperproject/skupper/pkg/domain/podman"
"github.com/spf13/cobra"
"gotest.tools/assert"
)

func TestNewClient(t *testing.T) {

const didNotExit = -99
tests := []struct {
scenario string
command string
args []string
cliFactory clientpodman.RestClientFactory
siteHandlerFactory podman.SiteHandlerFactory
currentSiteLoaded bool
exitCode int
stdoutContains string
}{
{
scenario: "init-invalid-endpoint",
command: "init",
args: []string{"/invalid/podman/endpoint"},
cliFactory: func(endpoint, basePath string) (*clientpodman.PodmanRestClient, error) {
return nil, fmt.Errorf("Podman endpoint is not available: %s", endpoint)
},
exitCode: 1,
stdoutContains: "Recommendation:",
},
{
scenario: "init-sitehandler-factory-error",
command: "init",
cliFactory: func(endpoint, basePath string) (*clientpodman.PodmanRestClient, error) {
return &clientpodman.PodmanRestClient{}, nil
},
siteHandlerFactory: func(endpoint string) (domain.SiteHandler, error) {
return nil, fmt.Errorf("unable to get sitehandler instance [mock]")
},
exitCode: 1,
stdoutContains: "error verifying existing skupper site",
},
{
scenario: "init-already-exists",
command: "init",
cliFactory: func(endpoint, basePath string) (*clientpodman.PodmanRestClient, error) {
return &clientpodman.PodmanRestClient{}, nil
},
siteHandlerFactory: func(endpoint string) (domain.SiteHandler, error) {
siteHandler := &siteHandlerMock{
getHook: func() (domain.Site, error) {
return &podman.Site{}, nil
},
}
return siteHandler, nil
},
currentSiteLoaded: false,
exitCode: 0,
stdoutContains: "Skupper has already been initialized for user",
},
{
scenario: "status-ok",
command: "status",
cliFactory: func(endpoint, basePath string) (*clientpodman.PodmanRestClient, error) {
return &clientpodman.PodmanRestClient{}, nil
},
siteHandlerFactory: func(endpoint string) (domain.SiteHandler, error) {
siteHandler := &siteHandlerMock{
getHook: func() (domain.Site, error) {
return &podman.Site{}, nil
},
}
return siteHandler, nil
},
currentSiteLoaded: true,
exitCode: didNotExit,
},
{
scenario: "status-not-enabled",
command: "status",
cliFactory: func(endpoint, basePath string) (*clientpodman.PodmanRestClient, error) {
return &clientpodman.PodmanRestClient{}, nil
},
siteHandlerFactory: func(endpoint string) (domain.SiteHandler, error) {
siteHandler := &siteHandlerMock{
getHook: func() (domain.Site, error) {
return nil, fmt.Errorf("skupper is not enabled")
},
}
return siteHandler, nil
},
currentSiteLoaded: false,
stdoutContains: "Skupper is not enabled for user",
exitCode: 0,
},
{
scenario: "version-not-enabled",
command: "version",
cliFactory: func(endpoint, basePath string) (*clientpodman.PodmanRestClient, error) {
return &clientpodman.PodmanRestClient{}, nil
},
siteHandlerFactory: func(endpoint string) (domain.SiteHandler, error) {
siteHandler := &siteHandlerMock{
getHook: func() (domain.Site, error) {
return nil, fmt.Errorf("skupper is not enabled")
},
}
return siteHandler, nil
},
exitCode: didNotExit,
},
}

var s *SkupperPodman
var lastExitCode int
stdout := new(bytes.Buffer)
resetSkupperPodman := func() {
s = &SkupperPodman{}
s.exit = func(code int) {
lastExitCode = code
}
s.output = stdout
lastExitCode = didNotExit
}

for _, test := range tests {
t.Run(test.scenario, func(t *testing.T) {
// reset test status
resetSkupperPodman()
// set factories
s.cliFactory = test.cliFactory
s.siteHandlerFactory = test.siteHandlerFactory
s.NewClient(&cobra.Command{
Use: test.command,
}, test.args)
assert.Equal(t, lastExitCode, test.exitCode)
assert.Assert(t, strings.Contains(stdout.String(), test.stdoutContains))
siteLoaded := s.currentSite != nil
assert.Assert(t, siteLoaded == test.currentSiteLoaded, "expected site to be loaded? %v", test.currentSiteLoaded)
})
}
}

type siteHandlerMock struct {
getHook func() (domain.Site, error)
}

func (sh *siteHandlerMock) Create(s domain.Site) error {
return fmt.Errorf("not implemented")
}

func (sh *siteHandlerMock) Get() (domain.Site, error) {
if sh.getHook == nil {
return nil, fmt.Errorf("not implemented")
}
return sh.getHook()
}

func (sh *siteHandlerMock) Delete() error {
return fmt.Errorf("not implemented")
}

func (sh *siteHandlerMock) Update() error {
return fmt.Errorf("not implemented")
}

func (sh *siteHandlerMock) RevokeAccess() error {
return fmt.Errorf("not implemented")
}
6 changes: 6 additions & 0 deletions pkg/domain/podman/site.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ func NewSitePodmanHandlerFromCli(cli *podman.PodmanRestClient) *SiteHandler {
}
}

type SiteHandlerFactory func(endpoint string) (domain.SiteHandler, error)

func NewSiteHandler(endpoint string) (domain.SiteHandler, error) {
return NewSitePodmanHandler(endpoint)
}

func NewSitePodmanHandler(endpoint string) (*SiteHandler, error) {
if endpoint == "" {
podmanCfg, err := NewPodmanConfigFileHandler().GetConfig()
Expand Down

0 comments on commit 628277f

Please sign in to comment.