Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass grains in minion startup event #54948

Merged
merged 3 commits into from Jan 14, 2020

Conversation

admd
Copy link
Contributor

@admd admd commented Oct 10, 2019

Note

The previous PR against develop branch #54087 has more discussion about the background for this change.

What does this PR do?

Adds a configuration option to have a selection of grains in the minion start event.

Reason behind this is better integration with Uyuni, especially when a lot of minions are starting in a short time frame - we need to get some minimal data (at least the machine_id) and getting it separately as a reaction to the minion start event (via a separate call or a startup state) generates considerable load on the master.

By default, there will be no grains passed to the event.

Previous Behavior

salt/minion/test-minion.tf.local/start	{
    "_stamp": "2019-08-01T10:46:22.840597", 
    "cmd": "_minion_event", 
    "data": "Minion test-minion.tf.local started at Thu Aug  1 12:46:22 2019", 
    "id": "test-minion.tf.local", 
    "pretag": null, 
    "tag": "salt/minion/test-minion.tf.local/start"
}

New Behavior

salt/minion/test-minion.tf.local/start	{
    "_stamp": "2019-08-01T10:46:58.696390", 
    "cmd": "_minion_event", 
    "data": "Minion test-minion.tf.local started at Thu Aug  1 12:46:58 2019", 
    "grains": {
        "uuid": "f4cd2a8c-eb73-11e9-81b4-2a2ae2dbcce4", 
        "machine_id": "21ee22c6906887c2dd6adeed5d404145"
    }, 
    "id": "test-minion.tf.local", 
    "pretag": null, 
    "tag": "salt/minion/test-minion.tf.local/start"
}

Tests written?

Yes

Commits signed with GPG?

No

@admd admd requested a review from as a code owner Oct 10, 2019
@ghost ghost requested a review from DmitryKuzmenko Oct 10, 2019
@admd admd force-pushed the pass-grains-in-minion-startup-event branch 2 times, most recently from 772336e to 581a0e0 Compare Oct 11, 2019
@admd
Copy link
Contributor Author

@admd admd commented Oct 23, 2019

@admd admd force-pushed the pass-grains-in-minion-startup-event branch from 581a0e0 to 198d7f6 Compare Oct 29, 2019
@brejoc
Copy link
Collaborator

@brejoc brejoc commented Oct 30, 2019

@saltstack/core-pr-reviewers Could you please take a look here? Seems like the tests aren't running.

@admd
Copy link
Contributor Author

@admd admd commented Nov 29, 2019

Any update here, please.

@moio
Copy link
Contributor

@moio moio commented Dec 17, 2019

@saltstack/core-pr-reviewers @DmitryKuzmenko this is now waiting since October, could you have a look in the near future by any chance?

@brejoc
Copy link
Collaborator

@brejoc brejoc commented Jan 14, 2020

Hey @s0undt3ch, thanks for taking a look here. Tests are looking good!

dwoz
dwoz approved these changes Jan 14, 2020
@dwoz dwoz merged commit 86183ba into saltstack:master Jan 14, 2020
49 checks passed
@max-arnold
Copy link
Contributor

@max-arnold max-arnold commented Jan 15, 2020

I just discovered that this feature (when enabled) will append grains to other events sent from a minion (not just the start event):

Both commands

sudo salt minion1 state.single event.send name='tag' data='{"data":"my event data"}'
sudo salt minion1 event.fire_master '{"data":"my event data"}' 'tag'

result in the following master event:

tag	{
    "_stamp": "2020-01-15T07:55:24.548188",
    "cmd": "_minion_event",
    "data": {
        "data": "my event data"
    },
    "grains": {
        "machine_id": "599118d5b9619443ac3166fb0e59349e",
        "uuid": "599118d5-b961-9443-ac31-66fb0e59349e"
    },
    "id": "minion1",
    "pretag": null,
    "tag": "tag"
}

def _fire_master_minion_start(self) seems to be better suited for this code.

@waynew
Copy link
Contributor

@waynew waynew commented Jan 15, 2020

@max-arnold just passing through on this PR - was that a suggestion that we should create a new PR to update this behavior? If so, would you mind opening an issue so it doesn't get lost here in the PR? Otherwise feel free to ignore this comment 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants