GT-49 | Show error when user tries to change value of "persistent" option (#366)

* Show error when user tries to change value of "persistent" option

* Add same test for docker, refactoring

* Fix after merge

Author: nikita-vanyasin

CHANGELOG.md

@@ -4,6 +4,7 @@
 - Add sanity check for pass-through args usage
 - Fix printing --starter.instance-up-timeout instead of hardcoded value
 - Fix context handling in WaitUntilStarterReady for tests
+- Show error when user tries to change value of "persistent" option
 
 ## [0.15.8](https://github.com/arangodb-helper/arangodb/tree/0.15.8) (2023-06-02)
 - Add passing ARANGODB_SERVER_DIR env variable when starting arangod instances

config.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2017-2022 ArangoDB GmbH, Cologne, Germany
+// Copyright 2017-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -28,6 +28,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
 	"gopkg.in/ini.v1"
+
+	"github.com/arangodb-helper/arangodb/service/options"
 )
 
 // loadCfgFromFile returns config loaded from file if file exists, nil otherwise
@@ -72,17 +74,20 @@ func trySetFlagFromConfig(flagName string, k *ini.Key, flagSets ...*pflag.FlagSe
 		return nil
 	}
 
-	prefix, targ, err := passthroughPrefixes.Lookup(flagName)
+	prefix, confPrefix, targ, err := passthroughPrefixes.Lookup(flagName)
 	if err != nil {
 		return errors.Wrapf(err, "invalid key %s", flagName)
 	}
-	if prefix != nil {
-		valuePtr := prefix.FieldSelector(passthroughOpts, targ)
+	if confPrefix != nil {
+		valuePtr := confPrefix.FieldSelector(passthroughOpts, targ)
 		if *valuePtr == nil {
 			*valuePtr = k.ValueWithShadows()
 		} else {
 			*valuePtr = append(*valuePtr, k.ValueWithShadows()...)
 		}
+		if options.IsPersistentOption(targ) {
+			passthroughOpts.PersistentOptions.Add(prefix, targ, valuePtr)
+		}
 		return nil
 	}
 	return fmt.Errorf("unknown key %s", flagName)
@@ -124,7 +129,7 @@ func loadFlagValuesFromConfig(cfgFilePath string, fs, persistentFs *pflag.FlagSe
 // example: --args.all.log.line-number --args.all.log.performance=true
 func sanityCheckPassThroughArgs(fs, persistentFs *pflag.FlagSet) {
 	sanityCheck := func(flag *pflag.Flag) {
-		if found, _, _ := passthroughPrefixes.Lookup(flag.Name); found != nil {
+		if _, found, _, _ := passthroughPrefixes.Lookup(flag.Name); found != nil {
 			if flag.Value == nil {
 				return
 			}

main.go

@@ -477,10 +477,19 @@ func cmdMainRun(cmd *cobra.Command, args []string) {
 	}
 
 	// Read setup.json (if exists)
-	bsCfg, clusterConfig, relaunch, _ := service.ReadSetupConfig(log, opts.starter.dataDir, bsCfg)
+	setupConfig, relaunch, _ := service.ReadSetupConfig(log, opts.starter.dataDir)
+	if relaunch {
+		oldOpts := setupConfig.Peers.PersistentOptions
+		newOpts := passthroughOpts.PersistentOptions
+		if err := oldOpts.ValidateCompatibility(&newOpts); err != nil {
+			log.Error().Err(err).Msg("Please check pass-through options")
+		}
+
+		bsCfg.LoadFromSetupConfig(setupConfig)
+	}
 
 	// Run the service
-	if err := svc.Run(rootCtx, bsCfg, clusterConfig, relaunch); err != nil {
+	if err := svc.Run(rootCtx, bsCfg, setupConfig.Peers, relaunch); err != nil {
 		log.Fatal().Err(err).Msg("Failed to run service")
 	}
 }

service/bootstrap_config.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2017 ArangoDB GmbH, Cologne, Germany
+// Copyright 2017-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,8 +17,6 @@
 //
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
-// Author Ewout Prangsma
-//
 
 package service
 
@@ -54,11 +52,11 @@ type BootstrapConfig struct {
 	RecoveryAgentID               string `json:"-"` // ID of the agent. Only set during recovery
 }
 
-func (bsCfg BootstrapConfig) JWTFolderDir() string {
+func (bsCfg *BootstrapConfig) JWTFolderDir() string {
 	return path.Join(bsCfg.DataDir, definitions.ArangodJWTSecretFolderName)
 }
 
-func (bsCfg BootstrapConfig) JWTFolderDirFile(f string) string {
+func (bsCfg *BootstrapConfig) JWTFolderDirFile(f string) string {
 	return path.Join(bsCfg.DataDir, definitions.ArangodJWTSecretFolderName, f)
 }
 
@@ -76,7 +74,7 @@ func (bsCfg *BootstrapConfig) Initialize() error {
 }
 
 // CreateTLSConfig creates a TLS config based on given bootstrap config
-func (bsCfg BootstrapConfig) CreateTLSConfig() (*tls.Config, error) {
+func (bsCfg *BootstrapConfig) CreateTLSConfig() (*tls.Config, error) {
 	if bsCfg.SslKeyFile == "" {
 		return nil, nil
 	}
@@ -90,7 +88,7 @@ func (bsCfg BootstrapConfig) CreateTLSConfig() (*tls.Config, error) {
 }
 
 // PeersNeeded returns the minimum number of peers needed for the given config.
-func (bsCfg BootstrapConfig) PeersNeeded() int {
+func (bsCfg *BootstrapConfig) PeersNeeded() int {
 	minServers := 1
 	switch {
 	case bsCfg.Mode.IsClusterMode():
@@ -105,3 +103,20 @@ func (bsCfg BootstrapConfig) PeersNeeded() int {
 	}
 	return minServers
 }
+
+// LoadFromSetupConfig loads important values from setup config file
+func (bsCfg *BootstrapConfig) LoadFromSetupConfig(cfg SetupConfigFile) {
+	// Reload data from config
+	bsCfg.ID = cfg.ID
+	if cfg.Mode != "" {
+		bsCfg.Mode = cfg.Mode
+	}
+	bsCfg.StartLocalSlaves = cfg.StartLocalSlaves
+	if cfg.SslKeyFile != "" {
+		bsCfg.SslKeyFile = cfg.SslKeyFile
+	}
+	if cfg.JwtSecret != "" {
+		bsCfg.JwtSecret = cfg.JwtSecret
+	}
+	bsCfg.AgencySize = cfg.Peers.AgencySize
+}

service/bootstrap_master.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2017 ArangoDB GmbH, Cologne, Germany
+// Copyright 2017-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,8 +17,6 @@
 //
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
-// Author Ewout Prangsma
-//
 
 package service
 
@@ -58,12 +56,10 @@ func (s *Service) bootstrapMaster(ctx context.Context, runner Runner, config Con
 	hasResilientSingle := boolFromRef(bsCfg.StartResilientSingle, s.mode.IsActiveFailoverMode())
 	hasSyncMaster := boolFromRef(bsCfg.StartSyncMaster, true) && config.SyncEnabled
 	hasSyncWorker := boolFromRef(bsCfg.StartSyncWorker, true) && config.SyncEnabled
-	s.myPeers.Initialize(
-		NewPeer(s.id, config.OwnAddress, s.announcePort, 0, config.DataDir,
-			hasAgent, hasDBServer, hasCoordinator, hasResilientSingle,
-			hasSyncMaster, hasSyncWorker,
-			s.IsSecure()),
-		bsCfg.AgencySize, storageEngine)
+	me := NewPeer(s.id, config.OwnAddress, s.announcePort, 0, config.DataDir,
+		hasAgent, hasDBServer, hasCoordinator, hasResilientSingle,
+		hasSyncMaster, hasSyncWorker, s.IsSecure())
+	s.myPeers.Initialize(me, bsCfg.AgencySize, storageEngine, s.cfg.Configuration.PersistentOptions)
 	s.learnOwnAddress = config.OwnAddress == ""
 
 	// Start HTTP listener

service/cluster_config.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2017-2021 ArangoDB GmbH, Cologne, Germany
+// Copyright 2017-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,9 +17,6 @@
 //
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
-// Author Ewout Prangsma
-// Author Tomasz Mielech
-//
 
 package service
 
@@ -38,16 +35,18 @@ import (
 	"github.com/arangodb/go-driver/jwt"
 
 	"github.com/arangodb-helper/arangodb/pkg/definitions"
+	"github.com/arangodb-helper/arangodb/service/options"
 )
 
-// ClusterConfig contains all the informtion of a cluster from a starter's point of view.
+// ClusterConfig contains all the information of a cluster from a starter's point of view.
 // When this type (or any of the types used in here) is changed, increase `SetupConfigVersion`.
 type ClusterConfig struct {
-	AllPeers            []Peer     `json:"Peers"` // All peers
-	AgencySize          int        // Number of agents
-	LastModified        *time.Time `json:"LastModified,omitempty"`        // Time of last modification
-	PortOffsetIncrement int        `json:"PortOffsetIncrement,omitempty"` // Increment of port offsets for peers on same address
-	ServerStorageEngine string     `json:ServerStorageEngine,omitempty"`  // Storage engine being used
+	AllPeers            []Peer                    `json:"Peers"` // All peers
+	AgencySize          int                       // Number of agents
+	LastModified        *time.Time                `json:"LastModified,omitempty"`        // Time of last modification
+	PortOffsetIncrement int                       `json:"PortOffsetIncrement,omitempty"` // Increment of port offsets for peers on same address
+	ServerStorageEngine string                    `json:"ServerStorageEngine,omitempty"` // Storage engine being used
+	PersistentOptions   options.PersistentOptions `json:"PersistentOptions,omitempty"`   // Options which were used during first start of DB and can't be changed anymore
 }
 
 // PeerByID returns a peer with given id & true, or false if not found.
@@ -83,11 +82,12 @@ func (p ClusterConfig) AllAgents() []Peer {
 }
 
 // Initialize a new cluster configuration
-func (p *ClusterConfig) Initialize(initialPeer Peer, agencySize int, storageEngine string) {
+func (p *ClusterConfig) Initialize(initialPeer Peer, agencySize int, storageEngine string, persistentOptions options.PersistentOptions) {
 	p.AllPeers = []Peer{initialPeer}
 	p.AgencySize = agencySize
 	p.PortOffsetIncrement = definitions.PortOffsetIncrementNew
 	p.ServerStorageEngine = storageEngine
+	p.PersistentOptions = persistentOptions
 	p.updateLastModified()
 }
 

service/local_slaves.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2017 ArangoDB GmbH, Cologne, Germany
+// Copyright 2017-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,8 +17,6 @@
 //
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
-// Author Ewout Prangsma
-//
 
 package service
 
@@ -69,15 +67,25 @@ func (s *Service) startLocalSlaves(wg *sync.WaitGroup, config Config, bsCfg Boot
 		os.MkdirAll(p.DataDir, 0755)
 
 		// Read existing setup.json (if any)
-		slaveBsCfg, clusterConfig, relaunch, _ := ReadSetupConfig(slaveLog, p.DataDir, slaveBsCfg)
+		setupConfig, relaunch, _ := ReadSetupConfig(slaveLog, p.DataDir)
+		if relaunch {
+			oldOpts := setupConfig.Peers.PersistentOptions
+			newOpts := config.Configuration.PersistentOptions
+			if err := oldOpts.ValidateCompatibility(&newOpts); err != nil {
+				s.log.Error().Err(err).Msg("Please check pass-through options")
+			}
+
+			slaveBsCfg.LoadFromSetupConfig(setupConfig)
+		}
+
 		slaveConfig := config // Create copy
 		slaveConfig.DataDir = p.DataDir
 		slaveConfig.MasterAddresses = []string{masterAddr}
 		slaveService := NewService(s.stopPeer.ctx, slaveLog, s.logService, slaveConfig, bsCfg, true)
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			if err := slaveService.Run(s.stopPeer.ctx, slaveBsCfg, clusterConfig, relaunch); err != nil {
+			if err := slaveService.Run(s.stopPeer.ctx, slaveBsCfg, setupConfig.Peers, relaunch); err != nil {
 				s.log.Error().Str("peer", p.ID).Err(err).Msg("Unable to start one of peers")
 			}
 		}()

service/options/options.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2021 ArangoDB GmbH, Cologne, Germany
+// Copyright 2021-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,8 +17,6 @@
 //
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
-// Author Adam Janikowski
-//
 
 package options
 
@@ -41,6 +39,8 @@ type Configuration struct {
 	AllSync      ConfigurationType
 	SyncMasters  ConfigurationType
 	SyncWorkers  ConfigurationType
+
+	PersistentOptions PersistentOptions
 }
 
 func NewConfiguration() Configuration {
@@ -212,19 +212,19 @@ type ConfigurationFlag struct {
 
 type ConfigurationPrefixes map[string]ConfigurationPrefix
 
-func (c ConfigurationPrefixes) Lookup(key string) (*ConfigurationPrefix, string, error) {
+func (c ConfigurationPrefixes) Lookup(key string) (string, *ConfigurationPrefix, string, error) {
 	for n, prefix := range c {
 		p := fmt.Sprintf("%s.", n)
 		if strings.HasPrefix(key, p) {
 			targ := strings.TrimPrefix(key, p)
 			if forbiddenOptions.IsForbidden(targ) {
-				return nil, "", fmt.Errorf("option --%s is essential to the starters behavior and cannot be overwritten", targ)
+				return n, nil, "", fmt.Errorf("option --%s is essential to the starters behavior and cannot be overwritten", targ)
 			}
 			prefix := prefix
-			return &prefix, targ, nil
+			return n, &prefix, targ, nil
 		}
 	}
-	return nil, "", nil
+	return "", nil, "", nil
 }
 
 func (c ConfigurationPrefixes) Parse(args ...string) (*Configuration, []ConfigurationFlag, error) {
@@ -239,12 +239,12 @@ func (c ConfigurationPrefixes) Parse(args ...string) (*Configuration, []Configur
 			continue
 		}
 
-		ckey := strings.TrimPrefix(arg, "--")
-		prefix, targ, err := c.Lookup(ckey)
+		cleanKey := strings.TrimPrefix(arg, "--")
+		prefix, confPrefix, targ, err := c.Lookup(cleanKey)
 		if err != nil {
 			return nil, nil, err
 		}
-		if prefix == nil {
+		if confPrefix == nil {
 			continue
 		}
 
@@ -254,14 +254,19 @@ func (c ConfigurationPrefixes) Parse(args ...string) (*Configuration, []Configur
 			flags[arg] = true
 		}
 
+		val := confPrefix.FieldSelector(&config, targ)
 		f = append(f, ConfigurationFlag{
 			Key:            arg,
-			CleanKey:       ckey,
+			CleanKey:       cleanKey,
 			Extension:      targ,
-			Usage:          prefix.Usage(targ),
-			Value:          prefix.FieldSelector(&config, targ),
-			DeprecatedHint: prefix.GetDeprecatedHint(targ),
+			Usage:          confPrefix.Usage(targ),
+			Value:          val,
+			DeprecatedHint: confPrefix.GetDeprecatedHint(targ),
 		})
+
+		if IsPersistentOption(targ) {
+			config.PersistentOptions.Add(prefix, targ, val)
+		}
 	}
 
 	return &config, f, nil