agent: when enable_central_service_config is enabled ensure agent reload doesn't revert check state to critical (#8747)

Likely introduced when #7345 landed.
This commit is contained in:
R.B. Boyer 2020-09-24 16:24:04 -05:00 committed by GitHub
parent 45609fccdf
commit 657995cc93
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 39 additions and 13 deletions

3
.changelog/8747.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
agent: when enable_central_service_config is enabled ensure agent reload doesn't revert check state to critical
```

View File

@ -1840,7 +1840,8 @@ func (a *Agent) AddServiceAndReplaceChecks(service *structs.NodeService, chkType
token: token, token: token,
replaceExistingChecks: true, replaceExistingChecks: true,
source: source, source: source,
}, a.snapshotCheckState()) snap: a.snapshotCheckState(),
})
} }
// AddService is used to add a service entry. // AddService is used to add a service entry.
@ -1859,12 +1860,13 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes []*structs.Che
token: token, token: token,
replaceExistingChecks: false, replaceExistingChecks: false,
source: source, source: source,
}, a.snapshotCheckState()) snap: a.snapshotCheckState(),
})
} }
// addServiceLocked adds a service entry to the service manager if enabled, or directly // addServiceLocked adds a service entry to the service manager if enabled, or directly
// to the local state if it is not. This function assumes the state lock is already held. // to the local state if it is not. This function assumes the state lock is already held.
func (a *Agent) addServiceLocked(req *addServiceRequest, snap map[structs.CheckID]*structs.HealthCheck) error { func (a *Agent) addServiceLocked(req *addServiceRequest) error {
req.fixupForAddServiceLocked() req.fixupForAddServiceLocked()
req.service.EnterpriseMeta.Normalize() req.service.EnterpriseMeta.Normalize()
@ -1882,7 +1884,7 @@ func (a *Agent) addServiceLocked(req *addServiceRequest, snap map[structs.CheckI
req.persistDefaults = nil req.persistDefaults = nil
req.persistServiceConfig = false req.persistServiceConfig = false
return a.addServiceInternal(req, snap) return a.addServiceInternal(req)
} }
// addServiceRequest is the union of arguments for calling both // addServiceRequest is the union of arguments for calling both
@ -1907,6 +1909,7 @@ type addServiceRequest struct {
token string token string
replaceExistingChecks bool replaceExistingChecks bool
source configSource source configSource
snap map[structs.CheckID]*structs.HealthCheck
} }
func (r *addServiceRequest) fixupForAddServiceLocked() { func (r *addServiceRequest) fixupForAddServiceLocked() {
@ -1920,7 +1923,7 @@ func (r *addServiceRequest) fixupForAddServiceInternal() {
} }
// addServiceInternal adds the given service and checks to the local state. // addServiceInternal adds the given service and checks to the local state.
func (a *Agent) addServiceInternal(req *addServiceRequest, snap map[structs.CheckID]*structs.HealthCheck) error { func (a *Agent) addServiceInternal(req *addServiceRequest) error {
req.fixupForAddServiceInternal() req.fixupForAddServiceInternal()
var ( var (
service = req.service service = req.service
@ -1932,6 +1935,7 @@ func (a *Agent) addServiceInternal(req *addServiceRequest, snap map[structs.Chec
token = req.token token = req.token
replaceExistingChecks = req.replaceExistingChecks replaceExistingChecks = req.replaceExistingChecks
source = req.source source = req.source
snap = req.snap
) )
// Pause the service syncs during modification // Pause the service syncs during modification
@ -3066,7 +3070,8 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
token: service.Token, token: service.Token,
replaceExistingChecks: false, // do default behavior replaceExistingChecks: false, // do default behavior
source: ConfigSourceLocal, source: ConfigSourceLocal,
}, snap) snap: snap,
})
if err != nil { if err != nil {
return fmt.Errorf("Failed to register service %q: %v", service.Name, err) return fmt.Errorf("Failed to register service %q: %v", service.Name, err)
} }
@ -3084,7 +3089,8 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
token: sidecarToken, token: sidecarToken,
replaceExistingChecks: false, // do default behavior replaceExistingChecks: false, // do default behavior
source: ConfigSourceLocal, source: ConfigSourceLocal,
}, snap) snap: snap,
})
if err != nil { if err != nil {
return fmt.Errorf("Failed to register sidecar for service %q: %v", service.Name, err) return fmt.Errorf("Failed to register sidecar for service %q: %v", service.Name, err)
} }
@ -3176,7 +3182,8 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig, snap map[structs.CheckI
token: p.Token, token: p.Token,
replaceExistingChecks: false, // do default behavior replaceExistingChecks: false, // do default behavior
source: source, source: source,
}, snap) snap: snap,
})
if err != nil { if err != nil {
return fmt.Errorf("failed adding service %q: %s", serviceID, err) return fmt.Errorf("failed adding service %q: %s", serviceID, err)
} }

View File

@ -3397,14 +3397,24 @@ func TestAgent_ReloadConfigOutgoingRPCConfig(t *testing.T) {
} }
func TestAgent_ReloadConfigAndKeepChecksStatus(t *testing.T) { func TestAgent_ReloadConfigAndKeepChecksStatus(t *testing.T) {
t.Parallel() t.Run("normal", func(t *testing.T) {
t.Parallel()
testAgent_ReloadConfigAndKeepChecksStatus(t, "")
})
t.Run("service manager", func(t *testing.T) {
t.Parallel()
testAgent_ReloadConfigAndKeepChecksStatus(t, "enable_central_service_config = true")
})
}
func testAgent_ReloadConfigAndKeepChecksStatus(t *testing.T, extraHCL string) {
dataDir := testutil.TempDir(t, "agent") // we manage the data dir dataDir := testutil.TempDir(t, "agent") // we manage the data dir
hcl := `data_dir = "` + dataDir + `" hcl := `data_dir = "` + dataDir + `"
enable_local_script_checks=true enable_local_script_checks=true
services=[{ services=[{
name="webserver1", name="webserver1",
check{id="check1", ttl="30s"} check{id="check1", ttl="30s"}
}]` }] ` + extraHCL
a := NewTestAgent(t, hcl) a := NewTestAgent(t, hcl)
defer a.Shutdown() defer a.Shutdown()
@ -3419,6 +3429,7 @@ func TestAgent_ReloadConfigAndKeepChecksStatus(t *testing.T) {
c := TestConfig(testutil.Logger(t), config.FileSource{Name: t.Name(), Format: "hcl", Data: hcl}) c := TestConfig(testutil.Logger(t), config.FileSource{Name: t.Name(), Format: "hcl", Data: hcl})
require.NoError(t, a.reloadConfigInternal(c)) require.NoError(t, a.reloadConfigInternal(c))
// After reload, should be passing directly (no critical state) // After reload, should be passing directly (no critical state)
for id, check := range a.State.Checks(nil) { for id, check := range a.State.Checks(nil) {
require.Equal(t, "passing", check.Status, "check %q is wrong", id) require.Equal(t, "passing", check.Status, "check %q is wrong", id)

View File

@ -87,7 +87,11 @@ func (s *ServiceManager) registerOnce(args *addServiceRequest) error {
s.agent.stateLock.Lock() s.agent.stateLock.Lock()
defer s.agent.stateLock.Unlock() defer s.agent.stateLock.Unlock()
err := s.agent.addServiceInternal(args, s.agent.snapshotCheckState()) if args.snap == nil {
args.snap = s.agent.snapshotCheckState()
}
err := s.agent.addServiceInternal(args)
if err != nil { if err != nil {
return fmt.Errorf("error updating service registration: %v", err) return fmt.Errorf("error updating service registration: %v", err)
} }
@ -128,7 +132,7 @@ func (s *ServiceManager) AddService(req *addServiceRequest) error {
req.persistService = nil req.persistService = nil
req.persistDefaults = nil req.persistDefaults = nil
req.persistServiceConfig = false req.persistServiceConfig = false
return s.agent.addServiceInternal(req, s.agent.snapshotCheckState()) return s.agent.addServiceInternal(req)
} }
var ( var (
@ -273,7 +277,8 @@ func (w *serviceConfigWatch) RegisterAndStart(
token: w.registration.token, token: w.registration.token,
replaceExistingChecks: w.registration.replaceExistingChecks, replaceExistingChecks: w.registration.replaceExistingChecks,
source: w.registration.source, source: w.registration.source,
}, w.agent.snapshotCheckState()) snap: w.agent.snapshotCheckState(),
})
if err != nil { if err != nil {
return fmt.Errorf("error updating service registration: %v", err) return fmt.Errorf("error updating service registration: %v", err)
} }