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

fix bash syntax and error handling #37

Merged
merged 1 commit into from
May 9, 2019

Conversation

odra
Copy link
Contributor

@odra odra commented May 8, 2019

  • fixes error handing
  • use posix compliant conditional syntax

@odra odra requested review from jessesarn, matskiv and pb82 May 8, 2019 10:50
Copy link
Contributor

@jessesarn jessesarn 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

maleck13 commented May 8, 2019

We will need an upgrade for this in the 1.4 upgrade playbook
Do we also need a change to installation to use this image?

@matskiv
Copy link
Member

matskiv commented May 8, 2019

Yeah, good point about the upgrade 👍
Maybe we can write something generic, that will update backup image version on each upgrade, so we don't have to do it each release?

PR looks good to me, I am building the image now, and testing it on my cluster in a moment.

@maleck13
Copy link

maleck13 commented May 8, 2019

Ok lets create a follow up for the upgrade piece and then get this merged and cherry-picked to the 1.4 branch

@matskiv
Copy link
Member

matskiv commented May 8, 2019

I finished the testing. Everything works(and fails) as expected.
This improvement actually helped to uncover an issue with 3Scale 2.5 postgresql backup - https://issues.jboss.org/browse/INTLY-2023

We will have to do a new release of backup image, and bump version in the installer.

@matskiv
Copy link
Member

matskiv commented May 8, 2019

I finished testing. Everything works(and fails) as expected. 👍

Improvement from this PR actually helped uncover another issue - https://issues.jboss.org/browse/INTLY-2023

For both installation and upgrade, we will have to do a new release of the backup image, and then bump the version in the installer

@odra odra merged commit 3326c40 into integr8ly:master May 9, 2019
@odra
Copy link
Contributor Author

odra commented May 9, 2019

does it need a cherry pick into the 1.4 branch?

@maleck13
Copy link

maleck13 commented May 9, 2019

yes also needs a follow up to deal with the upgrade

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

4 participants