From b32379a1be1dbc299fc8031697ecf3554adbc98f Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Tue, 20 May 2025 13:33:21 +0200 Subject: [PATCH] Make `is_acknowledged` a boolenum & add `is_sticky_acknowledgement` --- pkg/icingadb/types/acknowledgement_state.go | 61 ----------------- pkg/icingadb/v1/state.go | 61 ++++++++--------- schema/mysql/schema.sql | 6 +- schema/mysql/upgrades/1.4.0.sql | 10 +++ schema/pgsql/schema.sql | 7 +- schema/pgsql/upgrades/1.4.0.sql | 29 ++++++++ tests/history_test.go | 67 +++++++++++------- tests/object_sync_test.go | 75 +++++++++++++++++++++ tests/sla_test.go | 6 +- 9 files changed, 199 insertions(+), 123 deletions(-) delete mode 100644 pkg/icingadb/types/acknowledgement_state.go diff --git a/pkg/icingadb/types/acknowledgement_state.go b/pkg/icingadb/types/acknowledgement_state.go deleted file mode 100644 index 5a385653..00000000 --- a/pkg/icingadb/types/acknowledgement_state.go +++ /dev/null @@ -1,61 +0,0 @@ -package types - -import ( - "database/sql/driver" - "encoding" - "encoding/json" - "github.com/icinga/icinga-go-library/types" - "github.com/pkg/errors" -) - -// AcknowledgementState specifies an acknowledgement state (yes, no, sticky). -type AcknowledgementState uint8 - -// UnmarshalText implements the encoding.TextUnmarshaler interface. -func (as *AcknowledgementState) UnmarshalText(text []byte) error { - return as.UnmarshalJSON(text) -} - -// UnmarshalJSON implements the json.Unmarshaler interface. -func (as *AcknowledgementState) UnmarshalJSON(data []byte) error { - var i uint8 - if err := types.UnmarshalJSON(data, &i); err != nil { - return err - } - - a := AcknowledgementState(i) - if _, ok := acknowledgementStates[a]; !ok { - return badAcknowledgementState(data) - } - - *as = a - return nil -} - -// Value implements the driver.Valuer interface. -func (as AcknowledgementState) Value() (driver.Value, error) { - if v, ok := acknowledgementStates[as]; ok { - return v, nil - } else { - return nil, badAcknowledgementState(as) - } -} - -// badAcknowledgementState returns an error about a syntactically, but not semantically valid AcknowledgementState. -func badAcknowledgementState(s interface{}) error { - return errors.Errorf("bad acknowledgement state: %#v", s) -} - -// acknowledgementStates maps all valid AcknowledgementState values to their SQL representation. -var acknowledgementStates = map[AcknowledgementState]string{ - 0: "n", - 1: "y", - 2: "sticky", -} - -// Assert interface compliance. -var ( - _ encoding.TextUnmarshaler = (*AcknowledgementState)(nil) - _ json.Unmarshaler = (*AcknowledgementState)(nil) - _ driver.Valuer = AcknowledgementState(0) -) diff --git a/pkg/icingadb/v1/state.go b/pkg/icingadb/v1/state.go index 6ca8a4d0..5cd93401 100644 --- a/pkg/icingadb/v1/state.go +++ b/pkg/icingadb/v1/state.go @@ -8,34 +8,35 @@ import ( type State struct { EntityWithChecksum `json:",inline"` EnvironmentMeta `json:",inline"` - AcknowledgementCommentId types.Binary `json:"acknowledgement_comment_id"` - LastCommentId types.Binary `json:"last_comment_id"` - CheckAttempt uint32 `json:"check_attempt"` - CheckCommandline types.String `json:"check_commandline"` - CheckSource types.String `json:"check_source"` - SchedulingSource types.String `json:"scheduling_source"` - ExecutionTime float64 `json:"execution_time"` - HardState uint8 `json:"hard_state"` - InDowntime types.Bool `json:"in_downtime"` - AffectsChildren types.Bool `json:"affects_children"` - IsAcknowledged icingadbTypes.AcknowledgementState `json:"is_acknowledged"` - IsFlapping types.Bool `json:"is_flapping"` - IsHandled types.Bool `json:"is_handled"` - IsProblem types.Bool `json:"is_problem"` - IsReachable types.Bool `json:"is_reachable"` - LastStateChange types.UnixMilli `json:"last_state_change"` - LastUpdate types.UnixMilli `json:"last_update"` - Latency float64 `json:"latency"` - LongOutput types.String `json:"long_output"` - NextCheck types.UnixMilli `json:"next_check"` - NextUpdate types.UnixMilli `json:"next_update"` - Output types.String `json:"output"` - PerformanceData types.String `json:"performance_data"` - NormalizedPerformanceData types.String `json:"normalized_performance_data"` - PreviousSoftState uint8 `json:"previous_soft_state"` - PreviousHardState uint8 `json:"previous_hard_state"` - Severity uint16 `json:"severity"` - SoftState uint8 `json:"soft_state"` - StateType icingadbTypes.StateType `json:"state_type"` - CheckTimeout float64 `json:"check_timeout"` + AcknowledgementCommentId types.Binary `json:"acknowledgement_comment_id"` + LastCommentId types.Binary `json:"last_comment_id"` + CheckAttempt uint32 `json:"check_attempt"` + CheckCommandline types.String `json:"check_commandline"` + CheckSource types.String `json:"check_source"` + SchedulingSource types.String `json:"scheduling_source"` + ExecutionTime float64 `json:"execution_time"` + HardState uint8 `json:"hard_state"` + InDowntime types.Bool `json:"in_downtime"` + AffectsChildren types.Bool `json:"affects_children"` + IsAcknowledged types.Bool `json:"is_acknowledged"` + IsStickyAcknowledgement types.Bool `json:"is_sticky_acknowledgement"` + IsFlapping types.Bool `json:"is_flapping"` + IsHandled types.Bool `json:"is_handled"` + IsProblem types.Bool `json:"is_problem"` + IsReachable types.Bool `json:"is_reachable"` + LastStateChange types.UnixMilli `json:"last_state_change"` + LastUpdate types.UnixMilli `json:"last_update"` + Latency float64 `json:"latency"` + LongOutput types.String `json:"long_output"` + NextCheck types.UnixMilli `json:"next_check"` + NextUpdate types.UnixMilli `json:"next_update"` + Output types.String `json:"output"` + PerformanceData types.String `json:"performance_data"` + NormalizedPerformanceData types.String `json:"normalized_performance_data"` + PreviousSoftState uint8 `json:"previous_soft_state"` + PreviousHardState uint8 `json:"previous_hard_state"` + Severity uint16 `json:"severity"` + SoftState uint8 `json:"soft_state"` + StateType icingadbTypes.StateType `json:"state_type"` + CheckTimeout float64 `json:"check_timeout"` } diff --git a/schema/mysql/schema.sql b/schema/mysql/schema.sql index 6941db77..650774d6 100644 --- a/schema/mysql/schema.sql +++ b/schema/mysql/schema.sql @@ -309,7 +309,8 @@ CREATE TABLE host_state ( is_flapping enum('n', 'y') NOT NULL, is_overdue enum('n', 'y') NOT NULL, - is_acknowledged enum('n', 'y', 'sticky') NOT NULL, + is_acknowledged enum('n', 'y') NOT NULL, + is_sticky_acknowledgement enum('n', 'y') NOT NULL, acknowledgement_comment_id binary(20) DEFAULT NULL COMMENT 'comment.id', last_comment_id binary(20) DEFAULT NULL COMMENT 'comment.id', @@ -482,7 +483,8 @@ CREATE TABLE service_state ( is_flapping enum('n', 'y') NOT NULL, is_overdue enum('n', 'y') NOT NULL, - is_acknowledged enum('n', 'y', 'sticky') NOT NULL, + is_acknowledged enum('n', 'y') NOT NULL, + is_sticky_acknowledgement enum('n', 'y') NOT NULL, acknowledgement_comment_id binary(20) DEFAULT NULL COMMENT 'comment.id', last_comment_id binary(20) DEFAULT NULL COMMENT 'comment.id', diff --git a/schema/mysql/upgrades/1.4.0.sql b/schema/mysql/upgrades/1.4.0.sql index ee9d249f..a351cfab 100644 --- a/schema/mysql/upgrades/1.4.0.sql +++ b/schema/mysql/upgrades/1.4.0.sql @@ -2,10 +2,20 @@ ALTER TABLE host ADD COLUMN total_children int unsigned DEFAULT NULL AFTER check ALTER TABLE host_state ADD COLUMN affects_children enum('n', 'y') NOT NULL DEFAULT 'n' AFTER in_downtime; ALTER TABLE host_state MODIFY COLUMN affects_children enum('n', 'y') NOT NULL; +ALTER TABLE host_state ADD COLUMN is_sticky_acknowledgement enum('n', 'y') NOT NULL DEFAULT 'n' AFTER is_acknowledged; +ALTER TABLE host_state MODIFY COLUMN is_sticky_acknowledgement enum('n', 'y') NOT NULL; +UPDATE host_state SET is_sticky_acknowledgement = 'y', is_acknowledged = 'y' WHERE is_acknowledged = 'sticky'; +ALTER TABLE host_state MODIFY COLUMN is_acknowledged enum('n', 'y') NOT NULL; + ALTER TABLE service ADD COLUMN total_children int unsigned DEFAULT NULL AFTER check_retry_interval; ALTER TABLE service_state ADD COLUMN affects_children enum('n', 'y') NOT NULL DEFAULT 'n' AFTER in_downtime; ALTER TABLE service_state MODIFY COLUMN affects_children enum('n', 'y') NOT NULL; +ALTER TABLE service_state ADD COLUMN is_sticky_acknowledgement enum('n', 'y') NOT NULL DEFAULT 'n' AFTER is_acknowledged; +ALTER TABLE service_state MODIFY COLUMN is_sticky_acknowledgement enum('n', 'y') NOT NULL; +UPDATE service_state SET is_sticky_acknowledgement = 'y', is_acknowledged = 'y' WHERE is_acknowledged = 'sticky'; +ALTER TABLE service_state MODIFY COLUMN is_acknowledged enum('n', 'y') NOT NULL; + CREATE TABLE redundancy_group ( id binary(20) NOT NULL COMMENT 'sha1(name + all(member parent_name + timeperiod.name + states + ignore_soft_states))', environment_id binary(20) NOT NULL COMMENT 'environment.id', diff --git a/schema/pgsql/schema.sql b/schema/pgsql/schema.sql index ca56ce6d..f6113469 100644 --- a/schema/pgsql/schema.sql +++ b/schema/pgsql/schema.sql @@ -14,7 +14,6 @@ CREATE DOMAIN smalluint AS int CONSTRAINT between_0_and_65535 CHECK ( VALUE IS N CREATE DOMAIN tinyuint AS smallint CONSTRAINT between_0_and_255 CHECK ( VALUE IS NULL OR VALUE BETWEEN 0 AND 255 ); CREATE TYPE boolenum AS ENUM ( 'n', 'y' ); -CREATE TYPE acked AS ENUM ( 'n', 'y', 'sticky' ); CREATE TYPE state_type AS ENUM ( 'hard', 'soft' ); CREATE TYPE checkable_type AS ENUM ( 'host', 'service' ); CREATE TYPE comment_type AS ENUM ( 'comment', 'ack' ); @@ -423,7 +422,8 @@ CREATE TABLE host_state ( is_flapping boolenum NOT NULL DEFAULT 'n', is_overdue boolenum NOT NULL DEFAULT 'n', - is_acknowledged acked NOT NULL DEFAULT 'n', + is_acknowledged boolenum NOT NULL DEFAULT 'n', + is_sticky_acknowledgement boolenum NOT NULL DEFAULT 'n', acknowledgement_comment_id bytea20 DEFAULT NULL, last_comment_id bytea20 DEFAULT NULL, @@ -697,7 +697,8 @@ CREATE TABLE service_state ( is_flapping boolenum NOT NULL DEFAULT 'n', is_overdue boolenum NOT NULL DEFAULT 'n', - is_acknowledged acked NOT NULL DEFAULT 'n', + is_acknowledged boolenum NOT NULL DEFAULT 'n', + is_sticky_acknowledgement boolenum NOT NULL DEFAULT 'n', acknowledgement_comment_id bytea20 DEFAULT NULL, last_comment_id bytea20 DEFAULT NULL, diff --git a/schema/pgsql/upgrades/1.4.0.sql b/schema/pgsql/upgrades/1.4.0.sql index 6b21da6e..305b11a0 100644 --- a/schema/pgsql/upgrades/1.4.0.sql +++ b/schema/pgsql/upgrades/1.4.0.sql @@ -2,6 +2,35 @@ ALTER TABLE host ADD COLUMN total_children uint DEFAULT NULL; ALTER TABLE host_state ADD COLUMN affects_children boolenum NOT NULL DEFAULT 'n'; ALTER TABLE host_state ALTER COLUMN affects_children DROP DEFAULT; +ALTER TABLE host_state ADD COLUMN is_sticky_acknowledgement boolenum NOT NULL DEFAULT 'n'; +UPDATE host_state SET is_sticky_acknowledgement = 'y' WHERE is_acknowledged = 'sticky'; +-- The USING clause used below to typecast is_acknowledged to boolenum doesn't apply to default value [^1] +-- of a column, so we need to drop the DEFAULT constraint and then recreate it after the typecast. +-- [^1]: https://www.postgresql.org/docs/9.6/sql-altertable.html#notes +ALTER TABLE host_state ALTER COLUMN is_acknowledged DROP DEFAULT; +ALTER TABLE host_state ALTER COLUMN is_acknowledged TYPE boolenum USING ( + CASE is_acknowledged + WHEN 'y' THEN 'y'::boolenum + WHEN 'sticky' THEN 'y'::boolenum + ELSE 'n'::boolenum + END +); +ALTER TABLE host_state ALTER COLUMN is_acknowledged SET DEFAULT 'n'; + +ALTER TABLE service_state ADD COLUMN is_sticky_acknowledgement boolenum NOT NULL DEFAULT 'n'; +UPDATE service_state SET is_sticky_acknowledgement = 'y' WHERE is_acknowledged = 'sticky'; +ALTER TABLE service_state ALTER COLUMN is_acknowledged DROP DEFAULT; -- Same as above for host_state! +ALTER TABLE service_state ALTER COLUMN is_acknowledged TYPE boolenum USING ( + CASE is_acknowledged + WHEN 'y' THEN 'y'::boolenum + WHEN 'sticky' THEN 'y'::boolenum + ELSE 'n'::boolenum + END +); +ALTER TABLE service_state ALTER COLUMN is_acknowledged SET DEFAULT 'n'; + +DROP TYPE acked; + ALTER TABLE service ADD COLUMN total_children uint DEFAULT NULL; ALTER TABLE service_state ADD COLUMN affects_children boolenum NOT NULL DEFAULT 'n'; ALTER TABLE service_state ALTER COLUMN affects_children DROP DEFAULT; diff --git a/tests/history_test.go b/tests/history_test.go index 25b50b33..0adb9ebf 100644 --- a/tests/history_test.go +++ b/tests/history_test.go @@ -20,6 +20,7 @@ import ( "net/http" "sort" "strconv" + "strings" "testing" "text/template" "time" @@ -702,17 +703,24 @@ func assertStreamConsistency(t testing.TB, clients []*redis.Client, stream strin } } -func processCheckResult(t *testing.T, client *utils.Icinga2Client, hostname string, status int) time.Time { +// processCheckResult sends a passive check result to the Icinga2 API and returns the time of the check result. +// The provided hostOrServiceName must be either a host name or a service name in the format "host!service". +func processCheckResult(t *testing.T, client *utils.Icinga2Client, hostOrServiceName string, status int) time.Time { // Ensure that check results have distinct timestamps in millisecond resolution. time.Sleep(10 * time.Millisecond) output := utils.RandomString(8) - reqBody, err := json.Marshal(ActionsProcessCheckResultRequest{ - Type: "Host", - Filter: fmt.Sprintf(`host.name==%q`, hostname), - ExitStatus: status, - PluginOutput: output, - }) + req := ActionsProcessCheckResultRequest{ExitStatus: status, PluginOutput: output} + + if strings.ContainsRune(hostOrServiceName, '!') { + req.Type = "Service" + req.Service = hostOrServiceName + } else { + req.Type = "Host" + req.Host = hostOrServiceName + } + + reqBody, err := json.Marshal(req) require.NoError(t, err, "marshal request") response, err := client.PostJson("/v1/actions/process-check-result", bytes.NewBuffer(reqBody)) require.NoError(t, err, "process-check-result") @@ -723,29 +731,35 @@ func processCheckResult(t *testing.T, client *utils.Icinga2Client, hostname stri t.FailNow() } - response, err = client.GetJson("/v1/objects/hosts/" + hostname) - require.NoError(t, err, "get host: request") - require.Equal(t, 200, response.StatusCode, "get host: request") + response, err = client.GetJson(fmt.Sprintf("/v1/objects/%ss/%s", strings.ToLower(req.Type), hostOrServiceName)) + require.NoErrorf(t, err, "get %s: request", req.Type) + require.Equalf(t, 200, response.StatusCode, "get %s: request", req.Type) - var hosts ObjectsHostsResponse - err = json.NewDecoder(response.Body).Decode(&hosts) - require.NoError(t, err, "get host: parse response") + var checkables ObjectsCheckablesResponse + err = json.NewDecoder(response.Body).Decode(&checkables) + require.NoErrorf(t, err, "get %s: parse response", req.Type) - require.Equal(t, 1, len(hosts.Results), "there must be one host in the response") - host := hosts.Results[0] - require.Equal(t, output, host.Attrs.LastCheckResult.Output, - "last check result should be visible in host object") - require.Equal(t, status, host.Attrs.State, "state should match check result") + require.Equalf(t, 1, len(checkables.Results), "there must be one %s in the response", req.Type) + checkable := checkables.Results[0] + require.Equalf(t, output, checkable.Attrs.LastCheckResult.Output, + "last check result should be visible in %s object", req.Type) + require.Equal(t, status, checkable.Attrs.State, "state should match check result") - sec, nsec := math.Modf(host.Attrs.LastCheckResult.ExecutionEnd) + sec, nsec := math.Modf(checkable.Attrs.LastCheckResult.ExecutionEnd) return time.Unix(int64(sec), int64(nsec*1e9)) } type ActionsAcknowledgeProblemRequest struct { - Type string `json:"type"` - Filter string `json:"filter"` + Type string `json:"type"` + // Set either Host or Service, depending on the type of the check result + // or just the Filter, i.e. exactly one of Host, Service or Filter must be set. + Host string `json:"host,omitempty"` + Service string `json:"service,omitempty"` + Filter string `json:"filter,omitempty"` + Author string `json:"author"` Comment string `json:"comment"` + Sticky bool `json:"sticky"` } type ActionsAcknowledgeProblemResponse struct { @@ -778,8 +792,13 @@ type ActionsRemoveCommentRequest struct { } type ActionsProcessCheckResultRequest struct { - Type string `json:"type"` - Filter string `json:"filter"` + Type string `json:"type"` + // Set either Host or Service, depending on the type of the check result + // or just the Filter, i.e. exactly one of Host, Service or Filter must be set. + Host string `json:"host,omitempty"` + Service string `json:"service,omitempty"` + Filter string `json:"filter,omitempty"` + ExitStatus int `json:"exit_status"` PluginOutput string `json:"plugin_output"` } @@ -808,7 +827,7 @@ type ActionsScheduleDowntimeResponse struct { } `json:"results"` } -type ObjectsHostsResponse struct { +type ObjectsCheckablesResponse struct { Results []struct { Attrs struct { State int `json:"state"` diff --git a/tests/object_sync_test.go b/tests/object_sync_test.go index 7c3716b9..1d9ccb35 100644 --- a/tests/object_sync_test.go +++ b/tests/object_sync_test.go @@ -5,6 +5,7 @@ import ( "context" "database/sql" _ "embed" + "encoding/json" "fmt" "github.com/icinga/icinga-go-library/types" "github.com/icinga/icinga-testing/services" @@ -19,6 +20,7 @@ import ( "go.uber.org/zap" "golang.org/x/exp/slices" "io" + "net/http" "reflect" "sort" "strings" @@ -439,6 +441,79 @@ func TestObjectSync(t *testing.T) { }) }) + t.Run("State", func(t *testing.T) { + t.Parallel() + + for index := range 3 { + host := Host{ + Name: fmt.Sprintf("ack-test-host-%d", index), + CheckCommandName: "default-checkcommand", + EnableActiveChecks: false, + EnablePassiveChecks: true, + MaxCheckAttempts: 1, + CheckInterval: 300, + RetryInterval: 60, + } + host.DisplayName = host.Name + + t.Run("Verify-"+host.Name, func(t *testing.T) { + t.Parallel() + + client.CreateObject(t, "hosts", host.Name, map[string]interface{}{ + "attrs": makeIcinga2ApiAttributes(host, false), + }) + + eventually.Assert(t, func(t require.TestingT) { + verifyIcingaDbRow(t, db, host) + }, 20*time.Second, 1*time.Second) + + processCheckResult(t, client, host.Name, 1) + + isAcknowledged := index > 0 + sticky := index == 1 + if isAcknowledged { + req, err := json.Marshal(ActionsAcknowledgeProblemRequest{ + Type: "Host", + Host: host.Name, + Author: utils.RandomString(8), + Comment: utils.RandomString(8), + Sticky: sticky, + }) + require.NoError(t, err, "marshalling request body") + response, err := client.PostJson("/v1/actions/acknowledge-problem", bytes.NewBuffer(req)) + require.NoError(t, err, "acknowledge problem request") + require.Equal(t, http.StatusOK, response.StatusCode, "acknowledge problem request") + + var ackResponse ActionsAcknowledgeProblemResponse + err = json.NewDecoder(response.Body).Decode(&ackResponse) + _ = response.Body.Close() + + require.NoError(t, err, "decode acknowledge problem response") + require.Equal(t, 1, len(ackResponse.Results), "acknowledge problem response should've 1 result") + require.Equal(t, http.StatusOK, ackResponse.Results[0].Code, "acknowledge problem should've been successful") + } + + eventually.Assert(t, func(t require.TestingT) { + type Row struct { + IsAcknowledged types.Bool `db:"is_acknowledged"` + IsStickyAcknowledgement types.Bool `db:"is_sticky_acknowledgement"` + StateType string `db:"state_type"` + } + var row Row + err = db.Get(&row, `SELECT is_acknowledged, is_sticky_acknowledgement, state_type + FROM host_state + INNER JOIN host ON host_state.host_id = host.id + WHERE host.name = ?`, host.Name, + ) + require.NoError(t, err, "querying host state should not fail") + require.Equal(t, types.Bool{Bool: isAcknowledged, Valid: true}, row.IsAcknowledged, "host should be acknowledged") + require.Equal(t, types.Bool{Bool: sticky, Valid: true}, row.IsStickyAcknowledgement, "acknowledgement should be sticky") + require.Equal(t, "hard", row.StateType, "host should be in hard state") + }, 20*time.Second, 200*time.Millisecond) + }) + } + }) + t.Run("User", func(t *testing.T) { t.Parallel() diff --git a/tests/sla_test.go b/tests/sla_test.go index 597f5a05..d3e0a46c 100644 --- a/tests/sla_test.go +++ b/tests/sla_test.go @@ -49,7 +49,7 @@ func TestSla(t *testing.T) { var stateChanges []StateChange - processCheckResult := func(exitStatus int, isHard bool) *ObjectsHostsResponse { + processCheckResult := func(exitStatus int, isHard bool) *ObjectsCheckablesResponse { time.Sleep(10 * time.Millisecond) // ensure there is a bit of difference in ms resolution output := utils.UniqueName(t, "output") @@ -69,7 +69,7 @@ func TestSla(t *testing.T) { require.NoError(t, err, "get host: request") require.Equal(t, 200, response.StatusCode, "get host: request") - var hosts ObjectsHostsResponse + var hosts ObjectsCheckablesResponse err = json.NewDecoder(response.Body).Decode(&hosts) require.NoError(t, err, "get host: parse response") @@ -183,7 +183,7 @@ func TestSla(t *testing.T) { require.NoError(t, err, "get host: request") require.Equal(t, 200, response.StatusCode, "get host: request") - var hosts ObjectsHostsResponse + var hosts ObjectsCheckablesResponse err = json.NewDecoder(response.Body).Decode(&hosts) require.NoError(t, err, "get host: parse response")