CloudFormation signal program (issue #1581) #1728

mello7tre · 2021-09-01T13:14:46Z

Issue number:
#1581

Description of changes:
Created a new rust program, cfsignal to send signal to CloudFormation
Stack.
Program is a sort of cfn-signal
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-signal.html
but as cfn-signal need python cannot be used by bottlerocket.

cfsignal read configuration from a cfsignal.toml file configured reading
user-data, so it depends on settings-applier.service.
It cannot send a signal for a failure happening before
settings-applier.service and network-online.target are started.

It is able to send a failure signal for any other service starting from
(included):
activate-multi-user.service

It use systemctl action is-system-running with --wait option.
This way we can know if any service, after systemd boot process
finished, is in a failure status.

Detail
I have created a new setting named cloudformation.
It support the keys:

signal: boolean
stack_name: string
logical_resource_id: string

If signal is false (default) program do nothing.
Other way it excute systemctl --wait is-system-running to find out the system state.
Then it send the relative signal to the the stack_name stack.

Testing done:
Created and updated and AutoScalingGroup configured to wait signaling by created instances.
Tested that instance send SUCCESS signal if all service started successfully.

bash-5.0# systemctl status cfsignal
● cfsignal.service - Send signal to CloudFormation Stack
     Loaded: loaded (/x86_64-bottlerocket-linux-gnu/sys-root/usr/lib/systemd/system/cfsignal.service; enabled; vendor preset: enabled)
     Active: inactive (dead) since Thu 2022-02-24 03:20:40 UTC; 10min ago
    Process: 1578 ExecStart=/usr/bin/cfsignal (code=exited, status=0/SUCCESS)
   Main PID: 1578 (code=exited, status=0/SUCCESS)

Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] System status is: running [0]
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Connecting to IMDS
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Received meta-data/instance-id
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Received dynamic/instance-identity/document
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Region: "us-west-2" - InstanceID: "i-051f097c469cd6a7f" - Signal: "SUCCESS"
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal systemd[1]: cfsignal.service: Succeeded.

Tested that instance send FAILURE signal if a service after activate-multi-user.service fail.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

webern · 2021-09-03T18:40:12Z

Thank you for the PR @mello7tre! We are looking at it 👀 .

samuelkarp

@mello7tre thank you so much for putting this together! I took a look and had a few suggestions for changes. Please let me know if you have any questions about the suggestions. (And if you don't have the time to look at those suggestions, we can keep this open until you do or we can take a look at making those changes for you.)

packages/os/cfsignal.service

sources/cfsignal/Cargo.toml

sources/cfsignal/README.tpl

sources/cfsignal/src/main.rs

sources/models/shared-defaults/cf-signal.toml

sources/cfsignal/src/main.rs

sources/cfsignal/src/service_check.rs

sources/cfsignal/Cargo.toml

mello7tre · 2021-09-08T07:38:40Z

thanks @samuelkarp for the comments.
Give me some time and i will try to make the changes.
Mine only concern is about changing systemd.service Type, i have explained it better above.

packages/os/cfsignal.service

sources/cfsignal/Cargo.toml

sources/models/shared-defaults/cf-signal.toml

sources/cfsignal/src/main.rs

sources/cfsignal/src/error.rs

sources/cfsignal/src/cloudformation.rs

sources/cfsignal/src/system_check.rs

mello7tre · 2021-09-21T07:52:45Z

just pushed the requested changes.
I need more time to work on:
#1728 (comment)
#1728 (comment)

will came later...

jfrconley · 2021-12-20T22:31:05Z

Is there any movement on this issue? Waiting to adopt Bottlerocket until I can be sure that my asg will wait for instances to register with ECS

jpculp · 2021-12-21T01:26:03Z

All, we're sorry this has been left open for quite some time. From what I can tell it seems to be on the right track. @mello7tre, would you be able to re-base, squash the commits, and request re-review?

mello7tre · 2021-12-21T10:48:16Z

Rebased as requested.

etungsten · 2022-02-09T17:00:22Z

Hi @mello7tre, sorry about the delay. I'm gonna do my best and help you get this over the finish line. I took a look over the existing comments and resolved the ones that have been addressed.

The only comment that still needs addressing is #1728 (comment).

Another thing is we want to make sure cfsignal only runs on first boot as opposed to on every boot. We can achieve this by making cfsignal create an sentinel file at some path and the service unit can conditionally run based on the file's presence. The early-boot-config service is an example that does this:

bottlerocket/packages/os/early-boot-config.service

Lines 9 to 11 in 9749af9

    
           # We only want to run once, at first boot.  This file is created by early-boot-config 
        
           # after a successful run. 
        
           ConditionPathExists=!/var/lib/bottlerocket/early-boot-config.ran

And the sentinel file is created like so:

bottlerocket/sources/api/early-boot-config/src/main.rs

Lines 144 to 149 in 9749af9

    
           fs::write(MARKER_FILE, "").unwrap_or_else(|e| { 
        
               warn!( 
        
                   "Failed to create marker file {}, may unexpectedly run again: {}", 
        
                   MARKER_FILE, e 
        
               ) 
        
           });

Let me know what you think and how I can help.

And finally, since we're introducing new settings, we need to create a simple migration binary to migrate our settings datastore between OS versions. You can read more about migrations here and how to write them here. The closest example would be the add-network migration here:

bottlerocket/sources/api/migration/migrations/archived/v1.0.5/add-network-settings/src/main.rs

Lines 7 to 15 in 9749af9

    
           /// We added a set of settings for configuring service network behavior and their associated 
        
           /// configuration file. Remove the whole `settings.network`, `configuration-files.proxy-env` prefix 
        
           /// if we downgrade. 
        
           fn run() -> Result<()> { 
        
               migrate(AddPrefixesMigration(vec![ 
        
                   "settings.network", 
        
                   "configuration-files.proxy-env", 
        
               ])) 
        
           }

Of course, we can take care of this for you if you'd prefer not to dabble with it. Just let us know. Thanks again for your contribution!

mello7tre · 2022-02-10T13:25:23Z

Hi @etungsten, just pushed the requested changes.
Have a look if all is right.

etungsten

Looking good. We need to do a rebase to fix some minor conflicts and fix the migration crate path.

sources/cfsignal/src/cloudformation.rs

sources/cfsignal/src/main.rs

etungsten · 2022-02-10T16:25:37Z

sources/cfsignal/src/main.rs

+    let system_check = Box::new(SystemCheck {});
+    let system_status = system_check.system_running()?;
+    info!(
+        "System status is: {} [{}]",
+        system_status.status, system_status.exit_code
+    );
+
+    // run only if the opt-in flag is set
+    if config.should_signal {
+


Since we only do anything with the system check status if should_signal is set, we can have the check inside the if block.

Suggested change

let system_check = Box::new(SystemCheck {});

let system_status = system_check.system_running()?;

info!(

"System status is: {} [{}]",

system_status.status, system_status.exit_code

);

// run only if the opt-in flag is set

if config.should_signal {

// run only if the opt-in flag is set

if config.should_signal {

let system_check = Box::new(SystemCheck {});

let system_status = system_check.system_running()?;

info!(

"System status is: {} [{}]",

system_status.status, system_status.exit_code

);

Could be useful to know which would be the system status even without signaling it to CloudFormation, maybe to try it out in a dry mode or to debug.

packages/os/cfsignal.service

sources/api/migration/migrations/archived/v1.2.0/cfsignal/src/main.rs

sources/api/migration/migrations/archived/v1.2.0/cfsignal/Cargo.toml

sources/cfsignal/src/main.rs

mello7tre · 2022-02-11T10:03:30Z

applied requested changes
rebased on aeec3aa
squashed commits

mello7tre · 2022-02-11T11:12:01Z

i made a little mess rebasing, so i fixed it and force pushed the right "version"

etungsten

Nice! I'm gonna start asking others to take a look.

https://github.com/bottlerocket-os/bottlerocket/pull/1728/files#r803845979 still needs a quick fix.

Release.toml

sources/cfsignal/README.md

sources/cfsignal/src/system_check.rs

sources/cfsignal/src/error.rs

sources/api/migration/migrations/v1.7.0/add-cfsignal/Cargo.toml

mello7tre · 2022-02-11T17:51:51Z

Hi @etungsten, i think that to speed up the development is better if, from now on, you directly make all relevant changes.
So feel free to change it as you prefer, i just would like that this feature will be included in a future release.

Regards, Alberto.

etungsten · 2022-02-11T21:15:48Z

Push above addresses comments. Needed to label cfsignal with api_exec so it can write the sentinel file to a persistent storage location like /var/lib/bottlerocket.
Made sure cfsignal only runs on first boot. On subsequent boots, the cfsignal service unit never starts:

Feb 11 21:13:21 systemd[1]: Condition check resulted in Send signal to CloudFormation Stack being skipped.

webern

🎸

README.md

sources/cfsignal/Cargo.toml

sources/cfsignal/src/system_check.rs

sources/cfsignal/src/main.rs

i'm making the changes

sources/cfsignal/src/main.rs

sources/cfsignal/src/system_check.rs

sources/cfsignal/src/main.rs

packages/os/cfsignal.service

sources/cfsignal/src/error.rs

sources/cfsignal/src/main.rs

sources/models/shared-defaults/cf-signal.toml

etungsten · 2022-02-24T00:31:54Z

Push above and below addresses @arnaldo2792 's comments.

Tested changes and it works as expected:

bash-5.0# systemctl status cfsignal
● cfsignal.service - Send signal to CloudFormation Stack
     Loaded: loaded (/x86_64-bottlerocket-linux-gnu/sys-root/usr/lib/systemd/system/cfsignal.service; enabled; vendor preset: enabled)
     Active: inactive (dead) since Thu 2022-02-24 03:20:40 UTC; 10min ago
    Process: 1578 ExecStart=/usr/bin/cfsignal (code=exited, status=0/SUCCESS)
   Main PID: 1578 (code=exited, status=0/SUCCESS)

Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] System status is: running [0]
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Connecting to IMDS
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Received meta-data/instance-id
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Received dynamic/instance-identity/document
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal cfsignal[1578]: 03:20:40 [INFO] Region: "us-west-2" - InstanceID: "i-051f097c469cd6a7f" - Signal: "SUCCESS"
Feb 24 03:20:40 ip-192-168-16-4.us-west-2.compute.internal systemd[1]: cfsignal.service: Succeeded.

arnaldo2792

🦆

zmrow

🍨

Thanks!

bcressey

Nice! This looks ready to merge.

@mello7tre, I know this has been a long time coming! I really appreciate the contribution as well as your extraordinary patience. We're planning to ship it in the next release.

README.md

sources/models/src/aws-k8s-1.21/defaults.d/60-cf-signal.toml

mello7tre · 2022-03-04T08:25:53Z

Nice! This looks ready to merge.

@mello7tre, I know this has been a long time coming! I really appreciate the contribution as well as your extraordinary patience. We're planning to ship it in the next release.

Thanks, i am glad to have given a little contribution to the project.

etungsten · 2022-03-04T18:07:23Z

Push above rebases onto develop

etungsten · 2022-03-04T18:16:14Z

Push above addresses @bcressey 's comments.

Created a new rust program, cfsignal to send signal to CloudFormation Stack. Program is a sort of cfn-signal https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-signal.html but as cfn-signal need python cannot be used by bottlerocket. cfsignal read configuration from a cfsignal.toml file configured reading user-data, so it depends on settings-applier.service. It cannot send a signal for a failure happening before settings-applier.service and network-online.target are started. It is able to send a failure signal for any other service starting from (included): activate-multi-user.service It use systemctl action is-system-running with --wait option. This way we can know if any service, after systemd boot process finished, is in a failure status. Requested changes: * removed author * signal parameter renamed to should_signal (is more specific that should_send) * added README.md * removed commented out lines * use imdsclient in place of ec2_instance_metadata * refactor service_check.rs and renamed to system_check.rs use weak dependency (WantedBy)for cfsignal.service use tokio LTS, only with needed features restart command some code refactor * use directly signal_resource as function * code simplification in system_check.rs * use standard boilerplate for main function semaphore file and migration * Use semaphore file to only run on first boot * Add migration file for downgrading * client.signal_resource collapsed * Fix to packages/os/os.spec: toml file is not copyed (introduced during rebase) Readme changes

etungsten · 2022-03-04T22:03:01Z

Push above moves the migration to the v1.6.1 to v1.6.2 migration chain since we're planning this for v1.6.2 release.

etungsten · 2022-03-04T23:30:21Z

Tested migration again and it still works as expected

zmrow requested a review from samuelkarp September 7, 2021 16:31

samuelkarp reviewed Sep 8, 2021

View reviewed changes

arnaldo2792 reviewed Sep 15, 2021

View reviewed changes

packages/os/cfsignal.service Outdated Show resolved Hide resolved

bcressey reviewed Sep 20, 2021

View reviewed changes

mello7tre force-pushed the feat/cloudformation-signal branch from 495c6f5 to 18fef5b Compare December 21, 2021 10:33

etungsten self-assigned this Feb 8, 2022

etungsten suggested changes Feb 10, 2022

View reviewed changes

etungsten reviewed Feb 10, 2022

View reviewed changes

sources/cfsignal/src/main.rs Outdated Show resolved Hide resolved

sources/cfsignal/src/main.rs Outdated Show resolved Hide resolved

mello7tre force-pushed the feat/cloudformation-signal branch from a3a9f71 to 29a6ac8 Compare February 11, 2022 10:00

mello7tre force-pushed the feat/cloudformation-signal branch from 29a6ac8 to c746961 Compare February 11, 2022 11:10

mello7tre requested a review from etungsten February 11, 2022 11:17

etungsten suggested changes Feb 11, 2022

View reviewed changes

Release.toml Outdated Show resolved Hide resolved

sources/cfsignal/README.md Outdated Show resolved Hide resolved

sources/cfsignal/src/system_check.rs Outdated Show resolved Hide resolved

sources/cfsignal/src/error.rs Outdated Show resolved Hide resolved

etungsten requested review from bcressey, webern and arnaldo2792 February 11, 2022 17:29

etungsten previously requested changes Feb 11, 2022

View reviewed changes

sources/api/migration/migrations/v1.7.0/add-cfsignal/Cargo.toml Outdated Show resolved Hide resolved

etungsten force-pushed the feat/cloudformation-signal branch from c746961 to 01a5332 Compare February 11, 2022 21:06

etungsten force-pushed the feat/cloudformation-signal branch from 01a5332 to 4ee5959 Compare February 11, 2022 21:22

etungsten requested a review from cbgbt February 22, 2022 19:37

webern approved these changes Feb 22, 2022

View reviewed changes

README.md Show resolved Hide resolved

sources/cfsignal/Cargo.toml Show resolved Hide resolved

sources/cfsignal/src/system_check.rs Show resolved Hide resolved

sources/cfsignal/src/main.rs Show resolved Hide resolved

etungsten force-pushed the feat/cloudformation-signal branch 2 times, most recently from 4295732 to b4cd707 Compare February 23, 2022 01:16

cbgbt approved these changes Feb 23, 2022

View reviewed changes

arnaldo2792 reviewed Feb 23, 2022

View reviewed changes

etungsten force-pushed the feat/cloudformation-signal branch from b4cd707 to 90ed6c4 Compare February 24, 2022 00:28

etungsten force-pushed the feat/cloudformation-signal branch from 90ed6c4 to 046dbe7 Compare February 24, 2022 00:32

arnaldo2792 approved these changes Feb 24, 2022

View reviewed changes

cbgbt approved these changes Mar 1, 2022

View reviewed changes

zmrow approved these changes Mar 2, 2022

View reviewed changes

bcressey approved these changes Mar 4, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

sources/models/src/aws-k8s-1.21/defaults.d/60-cf-signal.toml Outdated Show resolved Hide resolved

etungsten force-pushed the feat/cloudformation-signal branch from 046dbe7 to 61d634f Compare March 4, 2022 18:07

etungsten force-pushed the feat/cloudformation-signal branch from 61d634f to 0a5d75f Compare March 4, 2022 18:16

bcressey approved these changes Mar 4, 2022

View reviewed changes

etungsten force-pushed the feat/cloudformation-signal branch from 0a5d75f to 526c606 Compare March 4, 2022 22:00

etungsten merged commit 92bdb47 into bottlerocket-os:develop Mar 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CloudFormation signal program (issue #1581) #1728

CloudFormation signal program (issue #1581) #1728

mello7tre commented Sep 1, 2021 •

edited by etungsten

Loading

webern commented Sep 3, 2021

samuelkarp left a comment

mello7tre commented Sep 8, 2021

mello7tre commented Sep 21, 2021

jfrconley commented Dec 20, 2021

jpculp commented Dec 21, 2021

mello7tre commented Dec 21, 2021

etungsten commented Feb 9, 2022 •

edited

Loading

mello7tre commented Feb 10, 2022

etungsten left a comment •

edited

Loading

etungsten Feb 10, 2022 •

edited

Loading

mello7tre Feb 10, 2022 •

edited

Loading

mello7tre commented Feb 11, 2022 •

edited

Loading

mello7tre commented Feb 11, 2022

etungsten left a comment

mello7tre commented Feb 11, 2022

etungsten commented Feb 11, 2022

webern left a comment

etungsten commented Feb 24, 2022 •

edited

Loading

arnaldo2792 left a comment

zmrow left a comment

bcressey left a comment

mello7tre commented Mar 4, 2022

etungsten commented Mar 4, 2022

etungsten commented Mar 4, 2022

etungsten commented Mar 4, 2022

etungsten commented Mar 4, 2022

CloudFormation signal program (issue #1581) #1728

CloudFormation signal program (issue #1581) #1728

Conversation

mello7tre commented Sep 1, 2021 • edited by etungsten Loading

webern commented Sep 3, 2021

samuelkarp left a comment

Choose a reason for hiding this comment

mello7tre commented Sep 8, 2021

mello7tre commented Sep 21, 2021

jfrconley commented Dec 20, 2021

jpculp commented Dec 21, 2021

mello7tre commented Dec 21, 2021

etungsten commented Feb 9, 2022 • edited Loading

mello7tre commented Feb 10, 2022

etungsten left a comment • edited Loading

Choose a reason for hiding this comment

etungsten Feb 10, 2022 • edited Loading

Choose a reason for hiding this comment

mello7tre Feb 10, 2022 • edited Loading

Choose a reason for hiding this comment

mello7tre commented Feb 11, 2022 • edited Loading

mello7tre commented Feb 11, 2022

etungsten left a comment

Choose a reason for hiding this comment

mello7tre commented Feb 11, 2022

etungsten commented Feb 11, 2022

webern left a comment

Choose a reason for hiding this comment

etungsten commented Feb 24, 2022 • edited Loading

arnaldo2792 left a comment

Choose a reason for hiding this comment

zmrow left a comment

Choose a reason for hiding this comment

bcressey left a comment

Choose a reason for hiding this comment

mello7tre commented Mar 4, 2022

etungsten commented Mar 4, 2022

etungsten commented Mar 4, 2022

etungsten commented Mar 4, 2022

etungsten commented Mar 4, 2022

mello7tre commented Sep 1, 2021 •

edited by etungsten

Loading

etungsten commented Feb 9, 2022 •

edited

Loading

etungsten left a comment •

edited

Loading

etungsten Feb 10, 2022 •

edited

Loading

mello7tre Feb 10, 2022 •

edited

Loading

mello7tre commented Feb 11, 2022 •

edited

Loading

etungsten commented Feb 24, 2022 •

edited

Loading