Skip to content

Commit

Permalink
feat: add DesiredReplicas inside State struct
Browse files Browse the repository at this point in the history 
For now only the `Kubernetes` provider benefits from this improvement as `Docker` and `Swarm` have hardcoded 1 value
  • Loading branch information
@acouvreur
acouvreur committed Nov 4, 2022
1 parent 8a85a32 commit 8096a4e
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 79 deletions.
2 changes: 1 addition & 1 deletion app/http/routes/strategies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func instanceStateToRenderOptionsRequestState(instanceState *instance.State) pag
Name: instanceState.Name,
Status: instanceState.Status,
CurrentReplicas: instanceState.CurrentReplicas,
DesiredReplicas: 1, //instanceState.DesiredReplicas,
DesiredReplicas: instanceState.DesiredReplicas,
Error: err,
}
}
Expand Down
8 changes: 4 additions & 4 deletions app/http/routes/strategies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@ func TestServeStrategy_ServeBlocking(t *testing.T) {
},
session: sessions.SessionState{
Instances: createMap([]*instance.State{
{Name: "nginx", Status: instance.NotReady, CurrentReplicas: 0},
{Name: "nginx", Status: instance.NotReady, CurrentReplicas: 0, DesiredReplicas: 1},
}),
},
},
expectedBody: `{"session":{"instances":[{"instance":{"name":"nginx","currentReplicas":0,"status":"not-ready"},"error":null}],"status":"not-ready"}}`,
expectedBody: `{"session":{"instances":[{"instance":{"name":"nginx","currentReplicas":0,"desiredReplicas":1,"status":"not-ready"},"error":null}],"status":"not-ready"}}`,
expectedHeaderKey: "X-Sablier-Session-Status",
expectedHeaderValue: "not-ready",
},
Expand All @@ -151,11 +151,11 @@ func TestServeStrategy_ServeBlocking(t *testing.T) {
},
session: sessions.SessionState{
Instances: createMap([]*instance.State{
{Name: "nginx", Status: instance.Ready, CurrentReplicas: 1},
{Name: "nginx", Status: instance.Ready, CurrentReplicas: 1, DesiredReplicas: 1},
}),
},
},
expectedBody: `{"session":{"instances":[{"instance":{"name":"nginx","currentReplicas":1,"status":"ready"},"error":null}],"status":"ready"}}`,
expectedBody: `{"session":{"instances":[{"instance":{"name":"nginx","currentReplicas":1,"desiredReplicas":1,"status":"ready"},"error":null}],"status":"ready"}}`,
expectedHeaderKey: "X-Sablier-Session-Status",
expectedHeaderValue: "ready",
},
Expand Down
31 changes: 10 additions & 21 deletions app/instance/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var Unrecoverable = "unrecoverable"
type State struct {
Name string `json:"name"`
CurrentReplicas int `json:"currentReplicas"`
DesiredReplicas int `json:"desiredReplicas"`
Status string `json:"status"`
Message string `json:"message,omitempty"`
}
Expand All @@ -17,54 +18,42 @@ func (instance State) IsReady() bool {
return instance.Status == Ready
}

func ErrorInstanceState(name string, err error) (State, error) {
func ErrorInstanceState(name string, err error, desiredReplicas int) (State, error) {
log.Error(err.Error())
return State{
Name: name,
CurrentReplicas: 0,
DesiredReplicas: desiredReplicas,
Status: Unrecoverable,
Message: err.Error(),
}, err
}

func UnrecoverableInstanceState(name string, message string) (State, error) {
func UnrecoverableInstanceState(name string, message string, desiredReplicas int) (State, error) {
log.Warn(message)
return State{
Name: name,
CurrentReplicas: 0,
DesiredReplicas: desiredReplicas,
Status: Unrecoverable,
Message: message,
}, nil
}

func ReadyInstanceState(name string) (State, error) {
return State{
Name: name,
CurrentReplicas: 1,
Status: Ready,
}, nil
}

func ReadyInstanceStateOfReplicas(name string, replicas int) (State, error) {
func ReadyInstanceState(name string, replicas int) (State, error) {
return State{
Name: name,
CurrentReplicas: replicas,
DesiredReplicas: replicas,
Status: Ready,
}, nil
}

func NotReadyInstanceState(name string) (State, error) {
return State{
Name: name,
CurrentReplicas: 0,
Status: NotReady,
}, nil
}

func NotReadyInstanceStateOfReplicas(name string, replicas int) (State, error) {
func NotReadyInstanceState(name string, currentReplicas int, desiredReplicas int) (State, error) {
return State{
Name: name,
CurrentReplicas: replicas,
CurrentReplicas: currentReplicas,
DesiredReplicas: desiredReplicas,
Status: NotReady,
}, nil
}
34 changes: 19 additions & 15 deletions app/providers/docker_classic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import (
)

type DockerClassicProvider struct {
Client client.ContainerAPIClient
Client client.ContainerAPIClient
desiredReplicas int
}

func NewDockerClassicProvider() (*DockerClassicProvider, error) {
Expand All @@ -21,7 +22,8 @@ func NewDockerClassicProvider() (*DockerClassicProvider, error) {
return nil, err
}
return &DockerClassicProvider{
Client: cli,
Client: cli,
desiredReplicas: 1,
}, nil
}

Expand All @@ -31,12 +33,13 @@ func (provider *DockerClassicProvider) Start(name string) (instance.State, error
err := provider.Client.ContainerStart(ctx, name, types.ContainerStartOptions{})

if err != nil {
return instance.ErrorInstanceState(name, err)
return instance.ErrorInstanceState(name, err, provider.desiredReplicas)
}

return instance.State{
Name: name,
CurrentReplicas: 0,
DesiredReplicas: provider.desiredReplicas,
Status: instance.NotReady,
}, err
}
Expand All @@ -48,12 +51,13 @@ func (provider *DockerClassicProvider) Stop(name string) (instance.State, error)
err := provider.Client.ContainerStop(ctx, name, nil)

if err != nil {
return instance.ErrorInstanceState(name, err)
return instance.ErrorInstanceState(name, err, provider.desiredReplicas)
}

return instance.State{
Name: name,
CurrentReplicas: 0,
DesiredReplicas: provider.desiredReplicas,
Status: instance.NotReady,
}, nil
}
Expand All @@ -64,38 +68,38 @@ func (provider *DockerClassicProvider) GetState(name string) (instance.State, er
spec, err := provider.Client.ContainerInspect(ctx, name)

if err != nil {
return instance.ErrorInstanceState(name, err)
return instance.ErrorInstanceState(name, err, provider.desiredReplicas)
}

// "created", "running", "paused", "restarting", "removing", "exited", or "dead"
switch spec.State.Status {
case "created", "paused", "restarting", "removing":
return instance.NotReadyInstanceState(name)
return instance.NotReadyInstanceState(name, 0, provider.desiredReplicas)
case "running":
if spec.State.Health != nil {
// // "starting", "healthy" or "unhealthy"
if spec.State.Health.Status == "healthy" {
return instance.ReadyInstanceState(name)
return instance.ReadyInstanceState(name, provider.desiredReplicas)
} else if spec.State.Health.Status == "unhealthy" {
if len(spec.State.Health.Log) >= 1 {
lastLog := spec.State.Health.Log[len(spec.State.Health.Log)-1]
return instance.UnrecoverableInstanceState(name, fmt.Sprintf("container is unhealthy: %s (%d)", lastLog.Output, lastLog.ExitCode))
return instance.UnrecoverableInstanceState(name, fmt.Sprintf("container is unhealthy: %s (%d)", lastLog.Output, lastLog.ExitCode), provider.desiredReplicas)
} else {
return instance.UnrecoverableInstanceState(name, "container is unhealthy: no log available")
return instance.UnrecoverableInstanceState(name, "container is unhealthy: no log available", provider.desiredReplicas)
}
} else {
return instance.NotReadyInstanceState(name)
return instance.NotReadyInstanceState(name, 0, provider.desiredReplicas)
}
}
return instance.ReadyInstanceState(name)
return instance.ReadyInstanceState(name, provider.desiredReplicas)
case "exited":
if spec.State.ExitCode != 0 {
return instance.UnrecoverableInstanceState(name, fmt.Sprintf("container exited with code \"%d\"", spec.State.ExitCode))
return instance.UnrecoverableInstanceState(name, fmt.Sprintf("container exited with code \"%d\"", spec.State.ExitCode), provider.desiredReplicas)
}
return instance.NotReadyInstanceState(name)
return instance.NotReadyInstanceState(name, 0, provider.desiredReplicas)
case "dead":
return instance.UnrecoverableInstanceState(name, "container in \"dead\" state cannot be restarted")
return instance.UnrecoverableInstanceState(name, "container in \"dead\" state cannot be restarted", provider.desiredReplicas)
default:
return instance.UnrecoverableInstanceState(name, fmt.Sprintf("container status \"%s\" not handled", spec.State.Status))
return instance.UnrecoverableInstanceState(name, fmt.Sprintf("container status \"%s\" not handled", spec.State.Status), provider.desiredReplicas)
}
}
25 changes: 22 additions & 3 deletions app/providers/docker_classic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -54,6 +55,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 1,
DesiredReplicas: 1,
Status: instance.Ready,
},
wantErr: false,
Expand All @@ -70,6 +72,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -86,6 +89,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.Unrecoverable,
Message: "container is unhealthy: curl http://localhost failed (1)",
},
Expand All @@ -103,6 +107,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 1,
DesiredReplicas: 1,
Status: instance.Ready,
},
wantErr: false,
Expand All @@ -119,6 +124,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -135,6 +141,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -151,6 +158,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -167,6 +175,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -183,6 +192,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.Unrecoverable,
Message: "container exited with code \"137\"",
},
Expand All @@ -200,6 +210,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.Unrecoverable,
Message: "container in \"dead\" state cannot be restarted",
},
Expand All @@ -217,6 +228,7 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.Unrecoverable,
Message: "container with name \"nginx\" was not found",
},
Expand All @@ -228,7 +240,8 @@ func TestDockerClassicProvider_GetState(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
provider := &DockerClassicProvider{
Client: tt.fields.Client,
Client: tt.fields.Client,
desiredReplicas: 1,
}

tt.fields.Client.On("ContainerInspect", mock.Anything, mock.Anything).Return(tt.containerSpec, tt.err)
Expand Down Expand Up @@ -271,6 +284,7 @@ func TestDockerClassicProvider_Stop(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.Unrecoverable,
Message: "container with name \"nginx\" was not found",
},
Expand All @@ -288,6 +302,7 @@ func TestDockerClassicProvider_Stop(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -297,7 +312,8 @@ func TestDockerClassicProvider_Stop(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
provider := &DockerClassicProvider{
Client: tt.fields.Client,
Client: tt.fields.Client,
desiredReplicas: 1,
}

tt.fields.Client.On("ContainerStop", mock.Anything, mock.Anything, mock.Anything).Return(tt.err)
Expand Down Expand Up @@ -340,6 +356,7 @@ func TestDockerClassicProvider_Start(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.Unrecoverable,
Message: "container with name \"nginx\" was not found",
},
Expand All @@ -357,6 +374,7 @@ func TestDockerClassicProvider_Start(t *testing.T) {
want: instance.State{
Name: "nginx",
CurrentReplicas: 0,
DesiredReplicas: 1,
Status: instance.NotReady,
},
wantErr: false,
Expand All @@ -366,7 +384,8 @@ func TestDockerClassicProvider_Start(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
provider := &DockerClassicProvider{
Client: tt.fields.Client,
Client: tt.fields.Client,
desiredReplicas: 1,
}

tt.fields.Client.On("ContainerStart", mock.Anything, mock.Anything, mock.Anything).Return(tt.err)
Expand Down
Loading

0 comments on commit 8096a4e

Please sign in to comment.