Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Fixes the installation of consul-backinator #799

Merged
merged 3 commits into from
May 7, 2019
Merged

Fixes the installation of consul-backinator #799

merged 3 commits into from
May 7, 2019

Conversation

JoshVanL
Copy link
Contributor

fixes #798
/assign @simonswine

Fix consul-backinator installation

@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/puppet Indicates a PR is affecting Puppet manifests labels Apr 24, 2019
@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2019
@JoshVanL JoshVanL mentioned this pull request Apr 24, 2019
Merged
simonswine
simonswine reviewed May 1, 2019
View reviewed changes
Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

Do we know why this changed?

puppet/modules/consul/spec/classes/install_spec.rb Outdated
@@ -56,13 +56,13 @@
should contain_file('/opt/consul-backinator-1.3').with(
:ensure => 'directory',
)
should contain_file('/opt/consul-backinator-1.3/consul-backinator').with(
should contain_file('/opt/consul-backinator-1.3/consul-backinator-1.3').with(
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is that change coming from?

Copy link
Contributor Author

@JoshVanL JoshVanL May 1, 2019 •
edited

Choose a reason for hiding this comment

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

@simonswine The binary is named consul-backinator-1.3 in the tar ball. When we had the creates before in the puppet resource it would create that file but would be empty.
I'm unsure the right way
to change it to just consul-backinator if that's what we what to do

Copy link
Contributor

Choose a reason for hiding this comment

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

This is quite a lot of change and I am not too sure how extract: false would have ever worked.

I would feel way better if we would rename the whole thing instead of having the version in there again.

Also happy to upgrade that component, if we touch it already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree about it not working. Happy to have another look and point it to a later version

@JoshVanL
Fixes the installation of consul-backinator
5ad728f 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Fixes consul puppet tests
e4c264e 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@jetstack-bot jetstack-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 1, 2019
@JoshVanL JoshVanL force-pushed the 798-fix-consul-backinator-install branch 3 times, most recently from 9cb8633 to 2f54604 Compare May 1, 2019 14:05
@simonswine simonswine assigned JoshVanL and unassigned simonswine May 1, 2019
@JoshVanL
Upgrade consul-backinator to 1.6.5 and extract to consul-backintor
90b3b3b 
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL JoshVanL force-pushed the 798-fix-consul-backinator-install branch from 2f54604 to 90b3b3b Compare May 1, 2019 14:34
@JoshVanL
Copy link
Contributor Author

JoshVanL commented May 1, 2019

/test puppet-tarmak-acceptance-ubuntu v1.11
/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL May 1, 2019
@simonswine
Copy link
Contributor

/test puppet-tarmak-acceptance-ubuntu v1.11

1 similar comment
@JoshVanL
Copy link
Contributor Author

JoshVanL commented May 1, 2019

/test puppet-tarmak-acceptance-ubuntu v1.11

@simonswine
Copy link
Contributor

Have you been successful at backing up:

systemctl start consul-backup.service
May 01 16:02:41 centos-72-x64 systemd[1]: Starting Backup consul KV store...
May 01 16:02:41 centos-72-x64 consul-backup.sh[5067]: 2019/05/01 16:02:41 [Error] Failed to backup key data: No keys found
May 01 16:02:41 centos-72-x64 systemd[1]: consul-backup.service: main process exited, code=exited, status=1/FAILURE
May 01 16:02:41 centos-72-x64 systemd[1]: Failed to start Backup consul KV store.

@simonswine
Copy link
Contributor

I made sure there is something stored in the KV store

@simonswine
Copy link
Contributor

/assign @JoshVanL
/unassign

@jetstack-bot jetstack-bot assigned JoshVanL and unassigned simonswine May 1, 2019
@JoshVanL
Copy link
Contributor Author

JoshVanL commented May 1, 2019

@simonswine Seems fine for me

May 01 14:57:03 ip-10-99-32-10.eu-west-1.compute.internal systemd[1]: Starting Backup consul KV store...
May 01 14:57:03 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: 2019/05/01 14:57:03 [Success] Backed up 234 keys from / to /tmp/consul-backup
May 01 14:57:03 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: 2019/05/01 14:57:03 [Success] Backed up 2 ACL tokens from  to /tmp/consul-backup-acls
May 01 14:57:03 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: Keep your backup and signature files in a safe place.
May 01 14:57:03 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: You will need both to restore your data.
May 01 14:57:04 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: [217B blob data]
May 01 14:57:04 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: [225B blob data]
May 01 14:57:05 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: [227B blob data]
May 01 14:57:05 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[18830]: [235B blob data]
May 01 14:57:05 ip-10-99-32-10.eu-west-1.compute.internal systemd[1]: Started Backup consul KV store.
May 01 16:27:07 ip-10-99-32-10.eu-west-1.compute.internal systemd[1]: Starting Backup consul KV store..

@JoshVanL
Copy link
Contributor Author

JoshVanL commented May 1, 2019

/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL May 1, 2019
@JoshVanL
Copy link
Contributor Author

JoshVanL commented May 2, 2019

/assign @MattiasGees

@MattiasGees
Copy link
Member

I tested this and it worked properly for me:

-- Logs begin at Tue 2019-05-07 11:31:42 UTC, end at Tue 2019-05-07 12:48:25 UTC. --
May 07 12:44:06 ip-10-99-32-10.eu-west-1.compute.internal systemd[1]: Starting Backup consul KV store...
May 07 12:44:06 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: 2019/05/07 12:44:06 [Success] Backed up 146 keys from /
May 07 12:44:06 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: 2019/05/07 12:44:06 [Success] Backed up 2 ACL tokens fr
May 07 12:44:06 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: Keep your backup and signature files in a safe place.
May 07 12:44:06 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: You will need both to restore your data.
May 07 12:44:07 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: [231B blob data]
May 07 12:44:07 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: [237B blob data]
May 07 12:44:08 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: [239B blob data]
May 07 12:44:08 ip-10-99-32-10.eu-west-1.compute.internal consul-backup.sh[17257]: [247B blob data]
May 07 12:44:08 ip-10-99-32-10.eu-west-1.compute.internal systemd[1]: Started Backup consul KV store.

@MattiasGees
Copy link
Member

/approve
/lgtm
/unassign
/assign @JoshVanL

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label May 7, 2019
@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoshVanL, MattiasGees

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot merged commit 7b521f8 into jetstack:master May 7, 2019
jetstack-bot added a commit that referenced this pull request May 27, 2019
@jetstack-bot
Merge pull request #800 from JoshVanL/automated-cherry-pick-of-#799-o…
814bceb 
…rigin-release-0.6

Automated cherry pick of #799
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@simonswine simonswine simonswine left review comments

Assignees

@simonswine simonswine

@JoshVanL JoshVanL

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/puppet Indicates a PR is affecting Puppet manifests dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consul-backinator not installing correctly
4 participants
@JoshVanL @simonswine @MattiasGees @jetstack-bot