Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make scale a *int as default is 1 (not 0) #481

Merged
merged 1 commit into from
Nov 21, 2023
Merged
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
4 changes: 1 addition & 3 deletions loader/full-struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
},
Deploy: &types.DeployConfig{
Mode: "replicated",
Replicas: uint64Ptr(6),
Replicas: intPtr(6),
Labels: map[string]string{"FOO": "BAR"},
RollbackConfig: &types.UpdateConfig{
Parallelism: uint64Ptr(3),
Expand Down Expand Up @@ -387,7 +387,6 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
Privileged: true,
ReadOnly: true,
Restart: types.RestartPolicyAlways,
Scale: 1,
Secrets: []types.ServiceSecretConfig{
{
Source: "secret1",
Expand Down Expand Up @@ -444,7 +443,6 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
DockerfileInline: "FROM alpine\nRUN echo \"hello\" > /world.txt\n",
},
Environment: types.MappingWithEquals{},
Scale: 1,
},
}
}
Expand Down
32 changes: 12 additions & 20 deletions loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ var sampleConfig = types.Config{
Networks: map[string]*types.ServiceNetworkConfig{
"with_me": nil,
},
Scale: 1,
},
{
Name: "bar",
Expand All @@ -231,7 +230,6 @@ var sampleConfig = types.Config{
Networks: map[string]*types.ServiceNetworkConfig{
"with_ipam": nil,
},
Scale: 1,
},
},
Networks: map[string]types.NetworkConfig{
Expand Down Expand Up @@ -817,7 +815,7 @@ networks:
Disable: true,
},
Deploy: &types.DeployConfig{
Replicas: uint64Ptr(555),
Replicas: intPtr(555),
UpdateConfig: &types.UpdateConfig{
Parallelism: uint64Ptr(555),
MaxFailureRatio: 3.14,
Expand All @@ -844,7 +842,6 @@ networks:
},
Privileged: true,
ReadOnly: true,
Scale: 1,
ShmSize: types.UnitBytes(2 * 1024 * 1024 * 1024),
StopGracePeriod: &typesDuration,
StdinOpen: true,
Expand Down Expand Up @@ -1054,14 +1051,18 @@ services:
`, map[string]string{"FOO_SCALE": "2"})

assert.NilError(t, err)
assert.Equal(t, project.Services[0].Scale, 2)
assert.Equal(t, *project.Services[0].Scale, 2)
}

func durationPtr(value time.Duration) *types.Duration {
result := types.Duration(value)
return &result
}

func intPtr(value int) *int {
return &value
}

func uint64Ptr(value uint64) *uint64 {
return &value
}
Expand Down Expand Up @@ -1583,7 +1584,6 @@ networks:
{
Name: "hello-world",
Image: "redis:alpine",
Scale: 1,
Networks: map[string]*types.ServiceNetworkConfig{
"network1": nil,
"network3": nil,
Expand Down Expand Up @@ -1634,7 +1634,6 @@ networks:
{
Name: "foo",
Image: "alpine",
Scale: 1,
Networks: map[string]*types.ServiceNetworkConfig{
"network1": {
Ipv4Address: "10.1.0.100",
Expand Down Expand Up @@ -1796,7 +1795,6 @@ secrets:
Source: "config",
},
},
Scale: 1,
Secrets: []types.ServiceSecretConfig{
{
Source: "secret",
Expand Down Expand Up @@ -1866,7 +1864,6 @@ secrets:
Source: "config",
},
},
Scale: 1,
Secrets: []types.ServiceSecretConfig{
{
Source: "secret",
Expand Down Expand Up @@ -1952,7 +1949,6 @@ func TestLoadWithExtends(t *testing.T) {
Target: "/var/lib/mysql",
Bind: &types.ServiceVolumeBind{CreateHostPath: true},
}},
Scale: 1,
},
}
assert.Check(t, is.DeepEqual(expServices, actual.Services))
Expand Down Expand Up @@ -1982,7 +1978,6 @@ func TestLoadWithExtendsWithContextUrl(t *testing.T) {
},
Environment: types.MappingWithEquals{},
Networks: map[string]*types.ServiceNetworkConfig{"default": nil},
Scale: 1,
},
}
assert.Check(t, is.DeepEqual(expServices, actual.Services))
Expand Down Expand Up @@ -2348,7 +2343,6 @@ func TestDeviceWriteBps(t *testing.T) {
Name: "foo",
Image: "busybox",
Environment: types.MappingWithEquals{},
Scale: 1,
BlkioConfig: &types.BlkioConfig{
DeviceReadBps: []types.ThrottleDevice{
{
Expand Down Expand Up @@ -2392,7 +2386,6 @@ volumes:
Name: "foo",
Image: "busybox",
Environment: types.MappingWithEquals{},
Scale: 1,
Volumes: []types.ServiceVolumeConfig{
{
Type: types.VolumeTypeVolume,
Expand All @@ -2418,7 +2411,6 @@ services:
Name: "x-foo",
Image: "busybox",
Environment: types.MappingWithEquals{},
Scale: 1,
},
})
}
Expand Down Expand Up @@ -2448,7 +2440,6 @@ services:
Name: "foo",
Image: "busybox",
Environment: types.MappingWithEquals{},
Scale: 1,
DependsOn: types.DependsOnConfig{"imported": {Condition: "service_started", Required: true}},
},
{
Expand All @@ -2459,7 +2450,6 @@ services:
filepath.Join(workingDir, "testdata", "subdir", "extra.env"),
},
Image: "nginx",
Scale: 1,
Volumes: []types.ServiceVolumeConfig{
{
Type: "bind",
Expand Down Expand Up @@ -2586,7 +2576,6 @@ services:
Name: "foo",
Image: "nginx",
Environment: types.MappingWithEquals{},
Scale: 1,
DependsOn: types.DependsOnConfig{
"bar": {Condition: types.ServiceConditionStarted, Required: true},
"baz": {Condition: types.ServiceConditionHealthy, Required: false},
Expand Down Expand Up @@ -2640,7 +2629,6 @@ services:
EnvFile: types.StringList{
filepath.Join(config.WorkingDir, "testdata", "remote", "env"),
},
Scale: 1,
Volumes: []types.ServiceVolumeConfig{
{
Type: types.VolumeTypeBind,
Expand Down Expand Up @@ -2833,8 +2821,12 @@ services:
memswap_limit: 640kb
`)
assert.NilError(t, err)
assert.Equal(t, project.Services[0].MemSwapLimit, types.UnitBytes(-1))
assert.Equal(t, project.Services[1].MemSwapLimit, types.UnitBytes(640*1024))
test1, err := project.GetService("test1")
assert.NilError(t, err)
assert.Equal(t, test1.MemSwapLimit, types.UnitBytes(-1))
test2, err := project.GetService("test2")
assert.NilError(t, err)
assert.Equal(t, test2.MemSwapLimit, types.UnitBytes(640*1024))
}

func TestBuildUlimits(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion loader/loader_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ services:
"image": "bar",
"command": "echo world",
"init": false,
"scale": 1,
},
},
})
Expand Down
20 changes: 0 additions & 20 deletions loader/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,6 @@ func Normalize(project *types.Project) error {
return err
}

err = relocateScale(&s)
if err != nil {
return err
}

inferImplicitDependencies(&s)

project.Services[i] = s
Expand Down Expand Up @@ -198,21 +193,6 @@ func setIfMissing(d types.DependsOnConfig, service string, dep types.ServiceDepe
return d
}

func relocateScale(s *types.ServiceConfig) error {
scale := uint64(s.Scale)
if scale > 1 {
logrus.Warn("`scale` is deprecated. Use the `deploy.replicas` element")
if s.Deploy == nil {
s.Deploy = &types.DeployConfig{}
}
if s.Deploy.Replicas != nil && *s.Deploy.Replicas != scale {
return errors.Wrap(errdefs.ErrInvalid, "can't use both 'scale' (deprecated) and 'deploy.replicas'")
}
s.Deploy.Replicas = &scale
}
return nil
}

// Resources with no explicit name are actually named by their key in map
func setNameFromKey(project *types.Project) {
for i, n := range project.Networks {
Expand Down
1 change: 0 additions & 1 deletion loader/normalize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func TestNormalizeNetworkNames(t *testing.T) {
"ZOT": nil,
},
},
Scale: 1,
},
},
}
Expand Down
8 changes: 8 additions & 0 deletions loader/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ func checkConsistency(project *types.Project) error {
return errors.Wrap(errdefs.ErrInvalid, fmt.Sprintf("service %q refers to undefined secret %s", s.Name, secret.Source))
}
}

if s.Scale != nil && s.Deploy != nil {
if s.Deploy.Replicas != nil && *s.Scale != *s.Deploy.Replicas {
return errors.Wrap(errdefs.ErrInvalid, "can't set distinct values on 'scale' and 'deploy.replicas'")
}
s.Deploy.Replicas = s.Scale
}

}

for name, secret := range project.Secrets {
Expand Down
2 changes: 0 additions & 2 deletions loader/with-version-struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func withVersionServices() []types.ServiceConfig {
"default": nil,
},
VolumesFrom: []string{"other"},
Scale: 1,
},
{
Name: "other",
Expand All @@ -56,7 +55,6 @@ func withVersionServices() []types.ServiceConfig {
Volumes: []types.ServiceVolumeConfig{
{Target: "/data", Type: "volume", Volume: &types.ServiceVolumeVolume{}},
},
Scale: 1,
},
}
}
Expand Down
3 changes: 0 additions & 3 deletions transform/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ import (

func transformService(data any, p tree.Path) (any, error) {
value := data.(map[string]any)
if _, ok := value["scale"]; !ok {
value["scale"] = 1 // TODO(ndeloof) we should make scale a *int
}
return transformMapping(value, p)
}

Expand Down
13 changes: 2 additions & 11 deletions types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ type ServiceConfig struct {
ReadOnly bool `yaml:"read_only,omitempty" json:"read_only,omitempty"`
Restart string `yaml:"restart,omitempty" json:"restart,omitempty"`
Runtime string `yaml:"runtime,omitempty" json:"runtime,omitempty"`
Scale int `yaml:"scale,omitempty" json:"scale,omitempty"`
Scale *int `yaml:"scale,omitempty" json:"scale,omitempty"`
Secrets []ServiceSecretConfig `yaml:"secrets,omitempty" json:"secrets,omitempty"`
SecurityOpt []string `yaml:"security_opt,omitempty" json:"security_opt,omitempty"`
ShmSize UnitBytes `yaml:"shm_size,omitempty" json:"shm_size,omitempty"`
Expand All @@ -138,19 +138,10 @@ type ServiceConfig struct {
func (s ServiceConfig) MarshalYAML() (interface{}, error) {
type t ServiceConfig
value := t(s)
value.Scale = 0 // deprecated, but default value "1" doesn't match omitempty
value.Name = "" // set during map to slice conversion, not part of the yaml representation
return value, nil
}

// MarshalJSON makes ServiceConfig implement json.Marshaller
func (s ServiceConfig) MarshalJSON() ([]byte, error) {
type t ServiceConfig
value := t(s)
value.Scale = 0 // deprecated, but default value "1" doesn't match omitempty
return json.Marshal(value)
}

// NetworksByPriority return the service networks IDs sorted according to Priority
func (s *ServiceConfig) NetworksByPriority() []string {
type key struct {
Expand Down Expand Up @@ -323,7 +314,7 @@ type LoggingConfig struct {
// DeployConfig the deployment configuration for a service
type DeployConfig struct {
Mode string `yaml:"mode,omitempty" json:"mode,omitempty"`
Replicas *uint64 `yaml:"replicas,omitempty" json:"replicas,omitempty"`
Replicas *int `yaml:"replicas,omitempty" json:"replicas,omitempty"`
Labels Labels `yaml:"labels,omitempty" json:"labels,omitempty"`
UpdateConfig *UpdateConfig `yaml:"update_config,omitempty" json:"update_config,omitempty"`
RollbackConfig *UpdateConfig `yaml:"rollback_config,omitempty" json:"rollback_config,omitempty"`
Expand Down
Loading