Pass the install event to the collector directly #150

Itxaka · 2023-09-25T10:35:01Z

Instead of saving files and whatnot, we can directly pass the event cc
data as a reader and have it merged into the final config

Itxaka · 2023-09-25T10:35:42Z

May fix kairos-io/kairos#1853 but as usual with the events, its a bit complicated to test if indeed its received that properly and parsing it properly......

@mudler any idea how can I manually test this?

codecov · 2023-09-25T10:39:25Z

Codecov Report

Merging #150 (3afce22) into main (9f46af2) will increase coverage by 0.12%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #150      +/-   ##
==========================================
+ Coverage   60.62%   60.74%   +0.12%     
==========================================
  Files          37       37              
  Lines        4480     4471       -9     
==========================================
  Hits         2716     2716              
+ Misses       1535     1526       -9     
  Partials      229      229

Files	Coverage Δ
internal/agent/install.go	`5.76% <0.00%> (+0.20%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Itxaka · 2023-09-25T10:52:03Z

Im guessing witht he provider you can run kairos-agent notify 'agent.install' 'JSON_PAYLOAD' ?

mauromorales · 2023-09-25T12:12:37Z

internal/agent/install.go

@@ -113,6 +113,10 @@ func Install(dir ...string) error {
 		if exists {
 			tmpdir, err := os.MkdirTemp("", "kairos-install-")
 			if err == nil {
+				// Add cloud config header if necessary
+				if !strings.Contains(cloudConfig, "#cloud-config") {
+					cloudConfig = fmt.Sprintf("#cloud-config\n%s", cloudConfig)


shouldn't we add the config in the config instead of here? otherwise the check for the config is not really that useful IMO

YEah but this comes from an event, which Its a map[string]string{} so I think the issue here its that when sending the data, it doesn not come with the cloud config header, so we are treating the data properly but when storing it, its just a file with no header, so the collector ignores it.

I think...I cannot test this properly. @jimmykarily can you tell me how to test this with your mock provider? I just want to see how the data arrives...

This is the object that its marshalled into json and pumped via the event:

type CloudInit struct { Install kairos.Install `yaml:"install,omitempty" json:"install,omitempty"` Options map[string]string `yaml:"options,omitempty" json:"options,omitempty"` Cluster clusterplugin.Cluster `yaml:"cluster,omitempty" json:"cluster,omitempty"` Stages map[string][]schema.Stage `yaml:"stages,omitempty" json:"stages,omitempty"` Stylus Stylus `yaml:"stylus,omitempty" json:"stylus,omitempty"` StylusPackage *string `yaml:"stylusPackage,omitempty" json:"stylusPackage,omitempty"` Env []string `yaml:"env,omitempty" json:"env,omitempty"` }

So ti kinds of make sense that what we receive its not with the header and what we save as cloud config needs the header to be read back up.

I wonder if we could make the collector read dirs and Reader objects directly so we could apss that data directly and skip the header stuff for those files.

Im wondering if we should just try to unmarshall the cloudConfig into an agentconfig.Config object directly and merge it on top of the base config which should be there already....

That would make this simpler and would make the event config end up on top of the others as expected....

I don't remember how I used the mock provider back then but I assume I just copied that to the directory where providers are expected and let kairos-agent run it.

btw, we could use this:

kairos-agent/pkg/config/config.go

Line 427 in 9f46af2

func AddHeader(header, data string) string {

jimmykarily · 2023-09-25T12:57:12Z

internal/agent/install.go

@@ -113,6 +113,10 @@ func Install(dir ...string) error {
 		if exists {
 			tmpdir, err := os.MkdirTemp("", "kairos-install-")
 			if err == nil {
+				// Add cloud config header if necessary
+				if !strings.Contains(cloudConfig, "#cloud-config") {


Maybe we should adapt and use this:

kairos-agent/pkg/config/config.go

Line 273 in 9f46af2

func HasHeader(userdata, head string) (bool, string) {

This is not a Config object so we cannot access that. This is just a plain string.

Itxaka · 2023-09-25T13:50:17Z

@jimmykarily using the new reader functionality of the collector.

collector commit is not merged into master!

Instead of saving files and whatnot, we can directly pass the event cc data as a reader and have it merged into the final config Signed-off-by: Itxaka <itxaka@kairos.io>

jimmykarily

Nice!

Signed-off-by: Itxaka <itxaka@kairos.io>

Itxaka · 2023-09-26T07:35:04Z

collector patch merged, sdk commit updated

Itxaka requested a review from a team September 25, 2023 10:35

mauromorales reviewed Sep 25, 2023

View reviewed changes

jimmykarily reviewed Sep 25, 2023

View reviewed changes

Itxaka force-pushed the fix_bus_cc branch from 2b9c619 to c2d851e Compare September 25, 2023 13:49

Itxaka changed the title ~~Check for cloud-config header in data coming from events~~ Pass the install event to the collector directly Sep 25, 2023

Itxaka requested review from jimmykarily and mauromorales September 25, 2023 13:49

Pass the install event to the collector directly

fabcf10

Instead of saving files and whatnot, we can directly pass the event cc data as a reader and have it merged into the final config Signed-off-by: Itxaka <itxaka@kairos.io>

Itxaka force-pushed the fix_bus_cc branch from c2d851e to fabcf10 Compare September 25, 2023 14:11

jimmykarily approved these changes Sep 26, 2023

View reviewed changes

mauromorales approved these changes Sep 26, 2023

View reviewed changes

Point sdk to proper commit

3afce22

Signed-off-by: Itxaka <itxaka@kairos.io>

Itxaka enabled auto-merge (squash) September 26, 2023 07:35

Itxaka merged commit 677254e into main Sep 26, 2023
5 checks passed

Itxaka deleted the fix_bus_cc branch September 26, 2023 07:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass the install event to the collector directly #150

Pass the install event to the collector directly #150

Itxaka commented Sep 25, 2023 •

edited

Loading

Itxaka commented Sep 25, 2023

codecov bot commented Sep 25, 2023 •

edited

Loading

Itxaka commented Sep 25, 2023

mauromorales Sep 25, 2023

Itxaka Sep 25, 2023

Itxaka Sep 25, 2023

Itxaka Sep 25, 2023

jimmykarily Sep 25, 2023

jimmykarily Sep 25, 2023

jimmykarily Sep 25, 2023

Itxaka Sep 25, 2023

Itxaka commented Sep 25, 2023

jimmykarily left a comment

Itxaka commented Sep 26, 2023

Pass the install event to the collector directly #150

Pass the install event to the collector directly #150

Conversation

Itxaka commented Sep 25, 2023 • edited Loading

Itxaka commented Sep 25, 2023

codecov bot commented Sep 25, 2023 • edited Loading

Codecov Report

Itxaka commented Sep 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Itxaka commented Sep 25, 2023

jimmykarily left a comment

Choose a reason for hiding this comment

Itxaka commented Sep 26, 2023

Itxaka commented Sep 25, 2023 •

edited

Loading

codecov bot commented Sep 25, 2023 •

edited

Loading