Skip to content

refactor sync-to-s3.sh per directory#623

Open
RobHooper wants to merge 1 commit into
mainfrom
sync-script-refactor
Open

refactor sync-to-s3.sh per directory#623
RobHooper wants to merge 1 commit into
mainfrom
sync-script-refactor

Conversation

@RobHooper
Copy link
Copy Markdown
Contributor

@RobHooper RobHooper commented May 12, 2026

This PR modifies the sync-to-s3.sh to sync one directory rather than many, this allows us to add directory specific features (exclude paths #618) without the feature being applied to all paths.
The main complexity of backing up directories has been moved into Salt creating a cron for each sync path and setting features as required - this should prove more extendable in the future.

Closes #618

Connected tasks:

  • Remove SYNC_DIRECTORIES from settings.local

@RobHooper RobHooper requested a review from jpmckinney May 12, 2026 14:20
@RobHooper RobHooper self-assigned this May 12, 2026

set +e
$AWS_CLI s3 sync "${AWS_ARGS[@]}" "$DIRECTORY" "s3://$S3_SYNC_BUCKET/$SAFENAME/"
set -e
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This final set -e does nothing, I intend for it to prevent human error keeping the flag active if the script was extended in the future.

Comment thread salt/aws/sync.sls
{%- for directory, entry in pillar.sync.directories|items %}
{%- set minute = (loop.index0 * 5) % 60 %}
{{minute}} 03,15 * * * root /home/sysadmin-tools/bin/sync-to-s3.sh {{ directory }}
{%- if entry and 'exclude' in entry %} --exclude "{{ entry.exclude }}"{% endif %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking: This puts the cron job onto two lines. Does cron read onto the second line?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe do (I think items is safe on None?):

{%- for option, value in entry|items %} --{{ option }} "{{ value }}"{% endfor %}

AWS_ARGS=(--only-show-errors --delete)

set +e
$AWS_CLI s3 sync "$DIRECTORY" "s3://$S3_SYNC_BUCKET/$SAFENAME/" --only-show-errors --delete 2>&1 | grep -v "You did not provide the number of bytes specified by the Content-Length HTTP header"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not need this anymore?

2>&1 | grep -v "You did not provide the number of bytes specified by the Content-Length HTTP header"

echo "Unknown option"
exit 1
;;
*)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit paranoid, as we control the Pillar data, and should only be setting real args. I think we can pass through args directly.

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.

S3 sync: Ignore requests.queue directories

2 participants