Skip to content

Update deb and rpm packages to create and assign permissions for /var/tempo#3657

Merged
mdisibio merged 7 commits intomainfrom
test-deb
May 8, 2024
Merged

Update deb and rpm packages to create and assign permissions for /var/tempo#3657
mdisibio merged 7 commits intomainfrom
test-deb

Conversation

@mdisibio
Copy link
Copy Markdown
Contributor

@mdisibio mdisibio commented May 8, 2024

What this PR does:
The motivation for this PR is to fix the deb and rpm Drone test process, but we decided it's also a good improvement that should be shipped in the packages.

The deb and rpm packages install Tempo as a systemd service, under a newly-created user tempo. However the services can't start out of the box because the tempo user lacks permissions to create and write to /var/tempo which is the default storage path. This updates the post install process to create and assign perms to /var/tempo. Unfortunately we couldn't pin down when this became a problem or what changed.

Note A tricky bit is that Drone only runs for tags and PRs of branches in the Tempo repo itself. If you PR is from a fork, it doesn't run. This made the failures go unnoticed for some time as most PRs are from forks.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Copy Markdown
Contributor

@zalegrala zalegrala left a comment

Choose a reason for hiding this comment

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

One comment, but looks good to me.

Comment thread tools/packaging/tempo-postinstall.sh Outdated
@mdisibio mdisibio changed the title test deb stuff Update deb and rpm packages to create and assign permissions for /var/tempo May 8, 2024
@mdisibio mdisibio marked this pull request as ready for review May 8, 2024 15:09
@mdisibio mdisibio merged commit 947de35 into main May 8, 2024
@mdisibio mdisibio deleted the test-deb branch May 8, 2024 16:28
@Enrice
Copy link
Copy Markdown

Enrice commented Oct 2, 2024

this should REALLY be fixed in the docker distribution too!

@zalegrala
Copy link
Copy Markdown
Contributor

@Enrice I'm not sure what you mean. Are you encountering an issue?

@Enrice
Copy link
Copy Markdown

Enrice commented Oct 2, 2024

My issue is that I would need to apply the workaround depicted in #3601 which I cannot do at the moment.

@zalegrala
Copy link
Copy Markdown
Contributor

If you'd like to give us more information about your environment, we can help you work through the issue. Consider joining the community slack. I'm zleslie in there and would be happy to take a look.

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.

3 participants