Re: [PATCH] s3-mirror: avoid potential unexpected shell-expansion, dedup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Note that I'd like to propose followup for this.

1. CMD1 could IMO safely exclude _only_ repomd.xml files.
2, CMD2 could safely _only_ sync/replace repomd.xml files.
3, then caches need to be invalidated, so we are sure we can --delete
4. CMD3 is keep as-is

The current approach is fine as well, but CMD2 must take unnecessarily
long time to calculate (do we have stats?).

There actually is slight risk, in current $CMD2, that 'repomd.xml' is
updated slightly faster than the rest of metadata.  So repomd.xml for
slight moment references non-existing metadata files...  which might
result in some unnecessary dnf fallbacks (that should succeed, but still).

WDYT?
Pavel

On Sunday, March 22, 2020 12:30:08 PM CET Pavel Raiskup wrote:
> Doing this has risk that '*' expands to directories:
> 
>     $> x='echo *'
>     $> $x
>     bin boot dev etc home
> 
> So it is better to use bash array (we have bash shebang anyways):
>     $> x=( echo '*' )
>     $> "${x[@]}"
>     *
> 
> Also de-duplicate few things so it is easier to concentrate on the code.
> ---
>  roles/s3-mirror/files/s3.sh | 288 +++++++++++++-----------------------
>  1 file changed, 106 insertions(+), 182 deletions(-)
> 
> diff --git a/roles/s3-mirror/files/s3.sh b/roles/s3-mirror/files/s3.sh
> index c70defb52..163e3dc36 100644
> --- a/roles/s3-mirror/files/s3.sh
> +++ b/roles/s3-mirror/files/s3.sh
> @@ -3,207 +3,131 @@
>  # LGPL
>  # Author: Rick Elrod <relrod@xxxxxxxxxx>
>  
> -# first run this command that syncs, but does not delete.
> +base_cmd=(
> +  aws s3 sync
> +  --no-follow-symlinks
> +  --exclude "*/.snapshot/*"
> +  --exclude "*/source/*"
> +  --exclude "*/SRPMS/*"
> +  --exclude "*/debug/*"
> +  --exclude "*/beta/*"
> +  --exclude "*/ppc/*"
> +  --exclude "*/ppc64/*"
> +  --exclude "*/repoview/*"
> +  --exclude "*/Fedora/*"
> +  --exclude "*/EFI/*"
> +  --exclude "*/core/*"
> +  --exclude "*/extras/*"
> +  --exclude "*/LiveOS/*"
> +  --exclude "*/development/rawhide/*"
> +  --exclude "*/releases/8/*"
> +  --exclude "*/releases/9/*"
> +  --exclude "*/releases/10/*"
> +  --exclude "*/releases/11/*"
> +  --exclude "*/releases/12/*"
> +  --exclude "*/releases/13/*"
> +  --exclude "*/releases/14/*"
> +  --exclude "*/releases/15/*"
> +  --exclude "*/releases/16/*"
> +  --exclude "*/releases/17/*"
> +  --exclude "*/releases/18/*"
> +  --exclude "*/releases/19/*"
> +  --exclude "*/releases/20/*"
> +  --exclude "*/releases/21/*"
> +  --exclude "*/releases/22/*"
> +  --exclude "*/releases/23/*"
> +  --exclude "*/releases/24/*"
> +  --exclude "*/releases/25/*"
> +  --exclude "*/releases/26/*"
> +  --exclude "*/releases/27/*"
> +  --exclude "*/releases/28/*"
> +  --exclude "*/releases/29/*"
> +  --exclude "*/updates/8/*"
> +  --exclude "*/updates/9/*"
> +  --exclude "*/updates/10/*"
> +  --exclude "*/updates/11/*"
> +  --exclude "*/updates/12/*"
> +  --exclude "*/updates/13/*"
> +  --exclude "*/updates/14/*"
> +  --exclude "*/updates/15/*"
> +  --exclude "*/updates/16/*"
> +  --exclude "*/updates/17/*"
> +  --exclude "*/updates/18/*"
> +  --exclude "*/updates/19/*"
> +  --exclude "*/updates/20/*"
> +  --exclude "*/updates/21/*"
> +  --exclude "*/updates/22/*"
> +  --exclude "*/updates/23/*"
> +  --exclude "*/updates/24/*"
> +  --exclude "*/updates/25/*"
> +  --exclude "*/updates/26/*"
> +  --exclude "*/updates/27/*"
> +  --exclude "*/updates/28/*"
> +  --exclude "*/updates/29/*"
> +  --exclude "*/updates/testing/8/*"
> +  --exclude "*/updates/testing/9/*"
> +  --exclude "*/updates/testing/10/*"
> +  --exclude "*/updates/testing/11/*"
> +  --exclude "*/updates/testing/12/*"
> +  --exclude "*/updates/testing/13/*"
> +  --exclude "*/updates/testing/14/*"
> +  --exclude "*/updates/testing/15/*"
> +  --exclude "*/updates/testing/16/*"
> +  --exclude "*/updates/testing/17/*"
> +  --exclude "*/updates/testing/18/*"
> +  --exclude "*/updates/testing/19/*"
> +  --exclude "*/updates/testing/20/*"
> +  --exclude "*/updates/testing/21/*"
> +  --exclude "*/updates/testing/22/*"
> +  --exclude "*/updates/testing/23/*"
> +  --exclude "*/updates/testing/24/*"
> +  --exclude "*/updates/testing/25/*"
> +  --exclude "*/updates/testing/26/*"
> +  --exclude "*/updates/testing/27/*"
> +  --exclude "*/updates/testing/28/*"
> +  --exclude "*/updates/testing/29/*"
> +)
> +
> +# First run this command that syncs, but does not delete.
>  # It also excludes repodata. 
> -CMD1="aws s3 sync                   \
> -  --exclude */repodata/*           \
> -  --exclude */.snapshot/*          \
> -  --exclude */source/*             \
> -  --exclude */SRPMS/*              \
> -  --exclude */debug/*              \
> -  --exclude */beta/*               \
> -  --exclude */ppc/*                \
> -  --exclude */ppc64/*              \
> -  --exclude */repoview/*           \
> -  --exclude */Fedora/*             \
> -  --exclude */EFI/*                \
> -  --exclude */core/*               \
> -  --exclude */extras/*             \
> -  --exclude */LiveOS/*             \
> -  --exclude */development/rawhide/* \
> -  --exclude */releases/8/*         \
> -  --exclude */releases/9/*         \
> -  --exclude */releases/10/*        \
> -  --exclude */releases/11/*        \
> -  --exclude */releases/12/*        \
> -  --exclude */releases/13/*        \
> -  --exclude */releases/14/*        \
> -  --exclude */releases/15/*        \
> -  --exclude */releases/16/*        \
> -  --exclude */releases/17/*        \
> -  --exclude */releases/18/*        \
> -  --exclude */releases/19/*        \
> -  --exclude */releases/20/*        \
> -  --exclude */releases/21/*        \
> -  --exclude */releases/22/*        \
> -  --exclude */releases/23/*        \
> -  --exclude */releases/24/*        \
> -  --exclude */releases/25/*        \
> -  --exclude */releases/26/*        \
> -  --exclude */releases/27/*        \
> -  --exclude */releases/28/*        \
> -  --exclude */releases/29/*        \
> -  --exclude */updates/8/*          \
> -  --exclude */updates/9/*          \
> -  --exclude */updates/10/*         \
> -  --exclude */updates/11/*         \
> -  --exclude */updates/12/*         \
> -  --exclude */updates/13/*         \
> -  --exclude */updates/14/*         \
> -  --exclude */updates/15/*         \
> -  --exclude */updates/16/*         \
> -  --exclude */updates/17/*         \
> -  --exclude */updates/18/*         \
> -  --exclude */updates/19/*         \
> -  --exclude */updates/20/*         \
> -  --exclude */updates/21/*         \
> -  --exclude */updates/22/*         \
> -  --exclude */updates/23/*         \
> -  --exclude */updates/24/*         \
> -  --exclude */updates/25/*         \
> -  --exclude */updates/26/*         \
> -  --exclude */updates/27/*         \
> -  --exclude */updates/28/*         \
> -  --exclude */updates/29/*         \
> -  --exclude */updates/testing/8/*  \
> -  --exclude */updates/testing/9/*  \
> -  --exclude */updates/testing/10/* \
> -  --exclude */updates/testing/11/* \
> -  --exclude */updates/testing/12/* \
> -  --exclude */updates/testing/13/* \
> -  --exclude */updates/testing/14/* \
> -  --exclude */updates/testing/15/* \
> -  --exclude */updates/testing/16/* \
> -  --exclude */updates/testing/17/* \
> -  --exclude */updates/testing/18/* \
> -  --exclude */updates/testing/19/* \
> -  --exclude */updates/testing/20/* \
> -  --exclude */updates/testing/21/* \
> -  --exclude */updates/testing/22/* \
> -  --exclude */updates/testing/23/* \
> -  --exclude */updates/testing/24/* \
> -  --exclude */updates/testing/25/* \
> -  --exclude */updates/testing/26/* \
> -  --exclude */updates/testing/27/* \
> -  --exclude */updates/testing/28/* \
> -  --exclude */updates/testing/29/* \
> -  --no-follow-symlinks             \
> -  "
> -  #--dryrun                         \
> +CMD1=( "${base_cmd[@]}" --exclude "*/repodata/*" )
>  
>  # Next we run this command which also includes repodata.
> -CMD2="aws s3 sync                   \
> -  --exclude */.snapshot/*          \
> -  --exclude */source/*             \
> -  --exclude */SRPMS/*              \
> -  --exclude */debug/*              \
> -  --exclude */beta/*               \
> -  --exclude */ppc/*                \
> -  --exclude */ppc64/*              \
> -  --exclude */repoview/*           \
> -  --exclude */Fedora/*             \
> -  --exclude */EFI/*                \
> -  --exclude */core/*               \
> -  --exclude */extras/*             \
> -  --exclude */LiveOS/*             \
> -  --exclude */development/rawhide/* \
> -  --exclude */releases/8/*         \
> -  --exclude */releases/9/*         \
> -  --exclude */releases/10/*        \
> -  --exclude */releases/11/*        \
> -  --exclude */releases/12/*        \
> -  --exclude */releases/13/*        \
> -  --exclude */releases/14/*        \
> -  --exclude */releases/15/*        \
> -  --exclude */releases/16/*        \
> -  --exclude */releases/17/*        \
> -  --exclude */releases/18/*        \
> -  --exclude */releases/19/*        \
> -  --exclude */releases/20/*        \
> -  --exclude */releases/21/*        \
> -  --exclude */releases/22/*        \
> -  --exclude */releases/23/*        \
> -  --exclude */releases/24/*        \
> -  --exclude */releases/25/*        \
> -  --exclude */releases/26/*        \
> -  --exclude */releases/27/*        \
> -  --exclude */releases/28/*        \
> -  --exclude */releases/29/*        \
> -  --exclude */updates/8/*          \
> -  --exclude */updates/9/*          \
> -  --exclude */updates/10/*         \
> -  --exclude */updates/11/*         \
> -  --exclude */updates/12/*         \
> -  --exclude */updates/13/*         \
> -  --exclude */updates/14/*         \
> -  --exclude */updates/15/*         \
> -  --exclude */updates/16/*         \
> -  --exclude */updates/17/*         \
> -  --exclude */updates/18/*         \
> -  --exclude */updates/19/*         \
> -  --exclude */updates/20/*         \
> -  --exclude */updates/21/*         \
> -  --exclude */updates/22/*         \
> -  --exclude */updates/23/*         \
> -  --exclude */updates/24/*         \
> -  --exclude */updates/25/*         \
> -  --exclude */updates/26/*         \
> -  --exclude */updates/27/*         \
> -  --exclude */updates/28/*         \
> -  --exclude */updates/29/*         \
> -  --exclude */updates/testing/8/*  \
> -  --exclude */updates/testing/9/*  \
> -  --exclude */updates/testing/10/* \
> -  --exclude */updates/testing/11/* \
> -  --exclude */updates/testing/12/* \
> -  --exclude */updates/testing/13/* \
> -  --exclude */updates/testing/14/* \
> -  --exclude */updates/testing/15/* \
> -  --exclude */updates/testing/16/* \
> -  --exclude */updates/testing/17/* \
> -  --exclude */updates/testing/18/* \
> -  --exclude */updates/testing/19/* \
> -  --exclude */updates/testing/20/* \
> -  --exclude */updates/testing/21/* \
> -  --exclude */updates/testing/22/* \
> -  --exclude */updates/testing/23/* \
> -  --exclude */updates/testing/24/* \
> -  --exclude */updates/testing/25/* \
> -  --exclude */updates/testing/26/* \
> -  --exclude */updates/testing/27/* \
> -  --exclude */updates/testing/28/* \
> -  --exclude */updates/testing/29/* \
> -  --no-follow-symlinks             \
> -  "
> -  #--dryrun                         \
> +CMD2=( "${base_cmd[@]}" )
> +
> +# Then we delete old RPMs and old metadata (but after invalidating caches).
> +CMD3=( "${base_cmd[@]}" --delete )
> +
> +S3_MIRROR=s3-mirror-us-west-1-02.fedoraproject.org
> +DIST_ID=E2KJMDC0QAJDMU
>  
>  # Sync EPEL
> -#echo $CMD /srv/pub/epel/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/epel/
> +#echo $CMD /srv/pub/epel/ s3://$S3_MIRROR/pub/epel/
>  echo "Starting EPEL sync at $(date)" >> /var/log/s3-mirror/timestamps
> -$CMD1 /srv/pub/epel/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/epel/
> -$CMD2 /srv/pub/epel/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/epel/
> +"${CMD1[@]}" /srv/pub/epel/ "s3://$S3_MIRROR/pub/epel/"
> +"${CMD2[@]}" /srv/pub/epel/ "s3://$S3_MIRROR/pub/epel/"
>  echo "Ending EPEL sync at $(date)" >> /var/log/s3-mirror/timestamps
>  for file in $(echo /srv/pub/epel/6/*/repodata/repomd.xml | sed 's#/srv##g'); do
> -  aws cloudfront create-invalidation --distribution-id E2KJMDC0QAJDMU --paths "$file"
> +  aws cloudfront create-invalidation --distribution-id "$DIST_ID" --paths "$file"
>  done
>  
>  for file in $(echo /srv/pub/epel/7/*/repodata/repomd.xml | sed 's#/srv##g'); do
> -  aws cloudfront create-invalidation --distribution-id E2KJMDC0QAJDMU --paths "$file"
> +  aws cloudfront create-invalidation --distribution-id "$DIST_ID" --paths "$file"
>  done
>  
>  for file in $(echo /srv/pub/epel/8/*/repodata/repomd.xml | sed 's#/srv##g'); do
> -  aws cloudfront create-invalidation --distribution-id E2KJMDC0QAJDMU --paths "$file"
> +  aws cloudfront create-invalidation --distribution-id "$DIST_ID" --paths "$file"
>  done
> -$CMD2 --delete /srv/pub/epel/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/epel/
> +"${CMD3[@]}" /srv/pub/epel/ "s3://$S3_MIRROR/pub/epel/"
>  
>  # Sync Fedora
> -#echo $CMD /srv/pub/fedora/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/fedora/
> +#echo $CMD /srv/pub/fedora/ s3://$S3_MIRROR/pub/fedora/
>  echo "Starting Fedora sync at $(date)" >> /var/log/s3-mirror/timestamps
> -$CMD1 /srv/pub/fedora/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/fedora/
> -$CMD2 /srv/pub/fedora/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/fedora/
> +"${CMD1[@]}" /srv/pub/fedora/ "s3://$S3_MIRROR/pub/fedora/"
> +"${CMD2[@]}" /srv/pub/fedora/ "s3://$S3_MIRROR/pub/fedora/"
>  echo "Ending Fedora sync at $(date)" >> /var/log/s3-mirror/timestamps
>  
>  for file in $(echo /srv/pub/fedora/linux/updates/*/*/*/repodata/repomd.xml | sed 's#/srv##g'); do
> -  aws cloudfront create-invalidation --distribution-id E2KJMDC0QAJDMU --paths "$file"
> +  aws cloudfront create-invalidation --distribution-id "$DIST_ID" --paths "$file"
>  done
> -$CMD2 --delete /srv/pub/fedora/ s3://s3-mirror-us-west-1-02.fedoraproject.org/pub/fedora/
> +"${CMD3[@]}" /srv/pub/fedora/ s3://$S3_MIRROR/pub/fedora/
> -- 
> 2.25.1
> _______________________________________________
> infrastructure mailing list -- infrastructure@xxxxxxxxxxxxxxxxxxxxxxx
> To unsubscribe send an email to infrastructure-leave@xxxxxxxxxxxxxxxxxxxxxxx
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/infrastructure@xxxxxxxxxxxxxxxxxxxxxxx
> 



_______________________________________________
infrastructure mailing list -- infrastructure@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to infrastructure-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/infrastructure@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Development]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]

  Powered by Linux