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.

Sign up for GitHub

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

(SUP-4215) Add creates param to archive resource #81

Merged
merged 1 commit into from
May 5, 2023
Merged

(SUP-4215) Add creates param to archive resource #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@
extract => true,
extract_command => 'tar xfz %s --strip-components=1',
extract_path => '/opt/influxdb',
creates => '/opt/influxdb/influxd',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you will have trouble when updating influxdb. The path doesn't contain the version number. When you update the version of influxdb, the creates path won't change and archive won't download the new version. In our prometheus module we always add the version to the path. Adding the version number will also provide a local history of files (when you don't clean it up), which makes rollbacks easier. I think that's something that should be changed in another PR. Vox Pupuli example: https://github.com/voxpupuli/puppet-prometheus/blob/master/manifests/daemon.pp#L108-L134

which will create something like this for the node exporter:

root@voxpupu ~ # tree /opt/node_exporter-1.*
/opt/node_exporter-1.3.1.linux-amd64
├── LICENSE
├── NOTICE
└── node_exporter
/opt/node_exporter-1.4.0.linux-amd64
├── LICENSE
├── NOTICE
└── node_exporter

0 directories, 3 files
root@voxpupu ~ # ls -la /usr/local/bin/node_exporter
lrwxrwxrwx 1 root root 50 Oct 10  2022 /usr/local/bin/node_exporter -> /opt/node_exporter-1.4.0.linux-amd64/node_exporter
root@voxpupu ~ #

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I was wondering how upgrades worked with archive sources. Essentially, there should be:

  • A versioned directory that the archive is extracted to, e.g. /opt/influxdb/influxd-2.6.1
  • A symlink for the main influxd binary that points to the latest archive directory, e.g. /usr/local/bin/influxd -> /opt/influxdb/influxd-2.6.1/influxd

Is that about right? I think we would need to make the startup script into a template so that it picks up the right binary. Maybe we should mark this "do not merge" until that is added, or just make a new PR with all of the changes? Although, I'm not sure if upgrading with archive sources works currently unless you delete /opt/influxdb first.

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO we should merge this PR, because it fixes the idempotency problem/works with newer puppet/archive version. Fixing the upgrade problem is a different issue. I would not make the startup script a template. When you add the symlink that's fine because /usr/local/bin is usually in $PATH. To be sure, you can update PATH="/opt/influxdb:${PATH}" to PATH="/usr/local/bin:${PATH}" in the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, if someone can approve it I'll merge this PR.

source => $archive_source,
cleanup => true,
require => File['/etc/influxdb', '/opt/influxdb'],
Expand Down
4 changes: 2 additions & 2 deletions metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
"version_requirement": ">= 7.0.0 < 9.0.0"
}
],
"pdk-version": "2.7.0",
"pdk-version": "2.7.1",
"template-url": "https://github.com/puppetlabs/pdk-templates#main",
"template-ref": "heads/main-0-g1ca5ee4"
"template-ref": "heads/main-0-gd05508f"
}