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

resources loop #46

Merged
merged 1 commit into from
May 22, 2019
Merged

resources loop #46

merged 1 commit into from
May 22, 2019

Conversation

odra
Copy link
Contributor

@odra odra commented May 21, 2019

Verification

  • Build the image
  • Run the backup job with the custom image
  • Check if all objects were properly backed up

@odra odra requested review from pb82 and matskiv May 21, 2019 13:19
@matskiv
Copy link
Member

matskiv commented May 21, 2019

Reviewing and testing now.

@@ -39,10 +39,11 @@ function backup_resource {
echo '---' > /tmp/${type}.yaml
for obj in $(oc get ${type} -n ${ns} | tr -s ' ' | cut -d ' ' -f 1 | tail -n +2); do
echo '-' >> /tmp/${type}.yaml
Copy link
Member

Choose a reason for hiding this comment

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

This line is no longer required.

Copy link
Member

Choose a reason for hiding this comment

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

and echo '---' > /tmp/${type}.yaml also.
/tmp/${type}.yaml doesn't have any real content besides dashes.

@matskiv
Copy link
Member

matskiv commented May 21, 2019

I looked at restoration SOP, and it looks like once all .gz archives are unpacked, we are expecting "$NS-*" files.
Current implementation creates enmasse-messagingusers.yaml.gz with this structure "tmp/messagingusers.*.yaml".

We should update SOP or update filenames and archive them without tmp/ folder.

@odra
Copy link
Contributor Author

odra commented May 21, 2019

Yeah the sop needs an update as well but I could add the ns var into the file name as well

@odra odra force-pushed the INTLY-2154_enmasse-obj-fix branch from 4dee173 to 78b00b7 Compare May 21, 2019 14:02
@odra
Copy link
Contributor Author

odra commented May 21, 2019

@matskiv sent a tiny update to remove the /tmp from the file path

echo "$(oc get ${type}/${obj} -n ${ns} -o yaml --export | sed 's/^/ /')" >> /tmp/${type}.yaml
cat /tmp/${type}.yaml | gzip > ${dest}/archives/${ns}-${type}.yaml.gz
rm -f /tmp/${type}.yaml
echo "$(oc get ${type}/${obj} -n ${ns} -o yaml --export)" > /tmp/${type}.${obj}.yaml
Copy link
Member

Choose a reason for hiding this comment

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

if we change output filename, to start with ${ns}-, we won't need SOP update IMHO

@odra odra force-pushed the INTLY-2154_enmasse-obj-fix branch from 78b00b7 to c6fff91 Compare May 21, 2019 14:11
@maleck13
Copy link

Once merged lets get this into 1.4 as well

@odra
Copy link
Contributor Author

odra commented May 22, 2019

still needs a final verification

done
(cd /tmp; find -name "*.yaml" | tar -cvzf "${dest}/archives/${ns}-${type}.yaml.gz" -T -)
Copy link
Member

Choose a reason for hiding this comment

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

This produces .tar.gz archive, and if one follows SOP and uses gunzip - it won't unpack this archive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok but all other components are using tar we should update the sop then

@odra odra force-pushed the INTLY-2154_enmasse-obj-fix branch from c6fff91 to fd763b0 Compare May 22, 2019 09:52
@odra odra force-pushed the INTLY-2154_enmasse-obj-fix branch from fd763b0 to 085c9f9 Compare May 22, 2019 10:35
Copy link
Member

@matskiv matskiv left a comment

Choose a reason for hiding this comment

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

Looks good.

@maleck13
Copy link

@odra ok to merge, release and cherry pick

@odra odra merged commit d8367f3 into integr8ly:master May 22, 2019
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

3 participants