Don't set TargetOptions before initialize

Following on from [28deaa4], if a deployment fails to initialize we
should avoid updating the `TargetOptions` in the state.

Author: kevinmcconnell

internal/server/router_test.go

@@ -222,8 +222,10 @@ func TestRouter_DeploymentsWithErrorsDoNotUpdateService(t *testing.T) {
 	serviceOptions := defaultServiceOptions
 	serviceOptions.Hosts = []string{"example.com"}
 
+	targetOptions := defaultTargetOptions
+
 	assert.NoFileExists(t, router.statePath)
-	require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, serviceOptions, defaultTargetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
+	require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, serviceOptions, targetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
 	ensureServiceIsHealthy()
 	require.FileExists(t, router.statePath)
 
@@ -239,23 +241,30 @@ func TestRouter_DeploymentsWithErrorsDoNotUpdateService(t *testing.T) {
 			sm.Set(service)
 		}
 		persistedOptions := sm.Get("service1").options
+		persistedTargetOptions := sm.Get("service1").targetOptions
+
 		assert.Equal(t, serviceOptions.TLSPrivateKeyPath, persistedOptions.TLSPrivateKeyPath)
 		assert.Equal(t, serviceOptions.TLSCertificatePath, persistedOptions.TLSCertificatePath)
 		assert.Equal(t, serviceOptions.TLSEnabled, persistedOptions.TLSEnabled)
 		assert.Equal(t, serviceOptions.ErrorPagePath, persistedOptions.ErrorPagePath)
+		assert.Equal(t, targetOptions.BufferRequests, persistedTargetOptions.BufferRequests)
 	}
 
 	t.Run("custom TLS that is not valid", func(t *testing.T) {
 		newServiceOptions := ServiceOptions{TLSEnabled: true, TLSCertificatePath: "not valid", TLSPrivateKeyPath: "not valid"}
-		require.Error(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, newServiceOptions, defaultTargetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
+		newTargetOptions := TargetOptions{BufferRequests: true, HealthCheckConfig: defaultHealthCheckConfig}
+
+		require.Error(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, newServiceOptions, newTargetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
 
 		ensureServiceIsHealthy()
 		ensureStateWasNotSaved()
 	})
 
 	t.Run("custom error pages that are not valid", func(t *testing.T) {
 		newServiceOptions := ServiceOptions{ErrorPagePath: "not valid"}
-		require.Error(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, newServiceOptions, defaultTargetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
+		newTargetOptions := TargetOptions{BufferRequests: true, HealthCheckConfig: defaultHealthCheckConfig}
+
+		require.Error(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, newServiceOptions, newTargetOptions, DefaultDeployTimeout, DefaultDrainTimeout))
 
 		ensureServiceIsHealthy()
 		ensureStateWasNotSaved()

internal/server/service.go

@@ -134,20 +134,17 @@ type Service struct {
 func NewService(name string, options ServiceOptions, targetOptions TargetOptions) (*Service, error) {
 	service := &Service{
 		name:            name,
-		targetOptions:   targetOptions,
 		pauseController: NewPauseController(),
 	}
 
-	if err := service.initialize(options); err != nil {
+	if err := service.initialize(options, targetOptions); err != nil {
 		return nil, err
 	}
 	return service, nil
 }
 
 func (s *Service) UpdateOptions(options ServiceOptions, targetOptions TargetOptions) error {
-	s.targetOptions = targetOptions
-
-	return s.initialize(options)
+	return s.initialize(options, targetOptions)
 }
 
 func (s *Service) Dispose() {
@@ -266,7 +263,6 @@ func (s *Service) UnmarshalJSON(data []byte) error {
 	s.name = ms.Name
 	s.pauseController = ms.PauseController
 	s.rolloutController = ms.RolloutController
-	s.targetOptions = ms.TargetOptions
 
 	activeTargets, err := NewTargetList(ms.ActiveTargets, ms.ActiveReaders, ms.TargetOptions)
 	if err != nil {
@@ -284,7 +280,7 @@ func (s *Service) UnmarshalJSON(data []byte) error {
 		s.rollout.MarkAllHealthy()
 	}
 
-	return s.initialize(ms.Options)
+	return s.initialize(ms.Options, ms.TargetOptions)
 }
 
 func (s *Service) Stop(drainTimeout time.Duration, message string) error {
@@ -325,7 +321,7 @@ func (s *Service) Resume() error {
 
 // Private
 
-func (s *Service) initialize(options ServiceOptions) error {
+func (s *Service) initialize(options ServiceOptions, targetOptions TargetOptions) error {
 	certManager, err := s.createCertManager(options)
 	if err != nil {
 		return err
@@ -337,6 +333,7 @@ func (s *Service) initialize(options ServiceOptions) error {
 	}
 
 	s.options = options
+	s.targetOptions = targetOptions
 	s.certManager = certManager
 	s.middleware = middleware