Refactor state store #4253

AndersonQ · 2024-02-14T07:48:06Z

What does this PR do?

It refactors the state store to:

Have an interface that better reflects what it actually does
- methods do not return a slice of action when only 1 action is involved
- action queue only accepts ScheduledAction
Add a Version field to the state store state
Test for saving 2 actions in the queue uses 2 upgrade actions as it's the only scheduled action so far.

Why is it important?

So the state store API reflects its actual usage and purpose.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
~~[ ] I have made corresponding changes to the documentation~~
~~[ ] I have made corresponding change to the default configuration files~~
I have added tests that prove my fix is effective or that my feature works
~~[ ] I have added an entry in ./changelog/fragments using the changelog tool~~
~~[ ] I have added an integration test or an E2E test~~

Related issues

Relates Scheduled upgrade fails with error parsing version "" if agent restarts before the upgrade starts #3912

Questions to ask yourself

How are we going to support this in production?
How are we going to measure its adoption?
How are we going to debug this?
What are the metrics I should take care of?
...

AndersonQ · 2024-02-14T14:27:22Z

internal/pkg/agent/storage/store/state_store.go

+type state struct {
+	ActionSerializer actionSerializer `json:"action,omitempty"`
+	AckToken         string           `json:"ack_token,omitempty"`
+	Queue            fleetapi.Actions `json:"action_queue,omitempty"`


Add a field to version the store

mergify · 2024-02-15T13:24:12Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 3912-refactor-state-store upstream/3912-refactor-state-store
git merge upstream/bugfix/3912-borken-stat-store
git push upstream 3912-refactor-state-store

elasticmachine · 2024-02-19T14:57:33Z

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

mergify · 2024-02-23T06:05:16Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 3912-refactor-state-store upstream/3912-refactor-state-store
git merge upstream/bugfix/3912-borken-stat-store
git push upstream 3912-refactor-state-store

.

rename its methods to be more meaningful and change the signature to reflect what actually happens.

pchila

Code looks fine except for some typos or nitpicks...

I wonder if we should go deeper with the refactor and have an ordered queue of actions with some retention mechanism (keep all the received but not yet run/unacked actions, keep at least 2 PolicyChangeActions, keep the UnEnroll action) in order to feed the dispatcher the necessary actions.

I realize that it is a big change but I think it would give us more flexibility and reliability as we have one more checkpoint between receiving an action from fleet and trying to run it.

Thoughts ?

internal/pkg/agent/cmd/run.go

internal/pkg/agent/storage/store/state_store.go

internal/pkg/agent/storage/store/state_store_test.go

internal/pkg/agent/application/gateway/fleet/fleet_gateway.go

elastic-sonarqube · 2024-03-05T14:55:31Z

Quality Gate passed

The SonarQube Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
68.3% 68.3% Coverage on New Code
0.0% 0.0% Duplication on New Code

See analysis details on SonarQube

pchila · 2024-03-06T13:42:22Z

internal/pkg/agent/storage/store/state_store.go

-		}
+	if st.Version != Version {
+		return nil, fmt.Errorf(
+			"invalid state store version, got %q isntead of %s",


Suggested change

"invalid state store version, got %q isntead of %s",

"invalid state store version, got %q instead of %q",

Fix action store -> state store migration tests (#4235) * use golden file for store migration test * remove withFile function * migrate take in a storage.Store instead of the storage's path. It's needed so we can set the encrypted store vault's path refactor state store (#4253) It modifies the state store API to match the current needs. update action model to match fleet-server schema (#4240) * simplify fleetapi.Actions.UnmarshalJSON * add test to ensure the state store is correctly loaded from disk * skip state store migration tests, they will be fixes on a follow-up PR as part of #3912 add migrations for action and state stores (#4305)

mergify bot assigned AndersonQ Feb 14, 2024

AndersonQ changed the title ~~WIP - refactor state store~~ Refactor state store Feb 14, 2024

AndersonQ changed the title ~~Refactor state store~~ [WIP] Refactor state store Feb 14, 2024

AndersonQ commented Feb 14, 2024

View reviewed changes

AndersonQ force-pushed the 3912-refactor-state-store branch from b6811f3 to 708f615 Compare February 19, 2024 11:22

AndersonQ added backport-skip skip-changelog Team:Elastic-Agent Label for the Agent team labels Feb 19, 2024

AndersonQ marked this pull request as ready for review February 19, 2024 14:57

AndersonQ requested a review from a team as a code owner February 19, 2024 14:57

AndersonQ requested review from blakerouse and pchila and removed request for a team February 19, 2024 14:57

AndersonQ changed the title ~~[WIP] Refactor state store~~ Refactor state store Feb 19, 2024

AndersonQ mentioned this pull request Feb 22, 2024

Match fleet server action model and use on JSON for serialisation #4240

Merged

3 tasks

AndersonQ added 11 commits February 23, 2024 07:14

remove stateSerializer and refactor actionSerialiser

92a8e9d

.

refactor state store

6223a23

rename its methods to be more meaningful and change the signature to reflect what actually happens.

add TODO

84518bd

refactor state store API

08c66a3

fix panic

c09dcad

create actionQueue to deserialize []fleetapi.ScheduledAction

cdf2b7f

add TODO

21a580f

add version field to store state

882caf7

restore save queue with two actions test

b671a23

doc improvement and cleanup

040874a

ops

cc98d01

AndersonQ force-pushed the 3912-refactor-state-store branch from 5eb5e3b to cc98d01 Compare February 23, 2024 06:14

pchila reviewed Mar 5, 2024

View reviewed changes

PR changes

0a47183

AndersonQ requested a review from pchila March 6, 2024 13:29

pchila reviewed Mar 6, 2024

View reviewed changes

pchila approved these changes Mar 6, 2024

View reviewed changes

AndersonQ merged commit a7b7bca into elastic:bugfix/3912-borken-stat-store Mar 7, 2024
6 of 9 checks passed

AndersonQ deleted the 3912-refactor-state-store branch March 12, 2024 06:10

AndersonQ mentioned this pull request Mar 26, 2024

Scheduled upgrade fails with error parsing version "" if agent restarts before the upgrade starts #3912

Closed

7 tasks

AndersonQ mentioned this pull request Apr 2, 2024

Fix and refactor the state store #4441

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor state store #4253

Refactor state store #4253

AndersonQ commented Feb 14, 2024 •

edited

Loading

AndersonQ Feb 14, 2024

mergify bot commented Feb 15, 2024

elasticmachine commented Feb 19, 2024

mergify bot commented Feb 23, 2024

pchila left a comment

elastic-sonarqube bot commented Mar 5, 2024

pchila Mar 6, 2024 •

edited

Loading

	"invalid state store version, got %q isntead of %s",
	"invalid state store version, got %q instead of %q",

Refactor state store #4253

Refactor state store #4253

Conversation

AndersonQ commented Feb 14, 2024 • edited Loading

What does this PR do?

Why is it important?

Checklist

Related issues

Questions to ask yourself

AndersonQ Feb 14, 2024

Choose a reason for hiding this comment

mergify bot commented Feb 15, 2024

elasticmachine commented Feb 19, 2024

mergify bot commented Feb 23, 2024

pchila left a comment

Choose a reason for hiding this comment

elastic-sonarqube bot commented Mar 5, 2024

Quality Gate passed

pchila Mar 6, 2024 • edited Loading

Choose a reason for hiding this comment

AndersonQ commented Feb 14, 2024 •

edited

Loading

pchila Mar 6, 2024 •

edited

Loading