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

Jump to bottom

Add build deb and rpm packages. #392

Merged
merged 4 commits into from
Dec 9, 2021
Merged

Add build deb and rpm packages. #392

merged 4 commits into from
Dec 9, 2021

Conversation

anton-bystrov
Copy link
Contributor

@anton-bystrov anton-bystrov commented Dec 7, 2021 •
edited
Loading

PMM-5598

Add building deb and rpm packages.

  • Tests passed.
  • Fix conflicts with target branch.
  • Update jira ticket description if needed.
  • Attach screenshots/console output to confirm new behavior to jira ticket, if applicable.

@it-percona-cla
Copy link

it-percona-cla commented Dec 7, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@anton-bystrov anton-bystrov requested review from percona-csalguero and removed request for BupycHuk December 7, 2021 07:57
denisok
denisok reviewed Dec 7, 2021
View reviewed changes
.scripts/postinst Outdated
getent group "$GROUPNAME" >/dev/null || groupadd -r "$GROUPNAME"
getent passwd "$USERNAME" >/dev/null || useradd -r -g "$GROUPNAME" -M -s /sbin/nologin -c "pmm service user" "$USERNAME"

systemctl daemon-reload > dev/null
Copy link
Contributor

Choose a reason for hiding this comment

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

is this required ? I am not specialist on deb packaging, but it sounds harsh.

Copy link

Choose a reason for hiding this comment

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

Debian has a lot of helper scripts. For example:
http://manpages.ubuntu.com/manpages/bionic/man1/deb-systemd-invoke.1p.html

The trick is that we need RPM and DEB packages together...

FPM can create systemd service itself but it seems that goreleaser NFPM can't...

.scripts/systemd/mongodb_exporter.service Outdated
Group=pmm

ExecStart=/usr/bin/mongodb_exporter \
--mongodb.uri=mongodb://user:[email protected]:27017/admin?ssl=true \
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting... this probably should come from the env variables or conf file.

.goreleaser.yml Outdated
bindir: /usr/bin
replacements:
amd64: 64-bit
darwin: macOS
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed for deb and rpm?

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 think this naming is more beautiful...no more.

nikita-b
nikita-b reviewed Dec 7, 2021
View reviewed changes
.scripts/postinst Outdated
USERNAME="pmm"

getent group "$GROUPNAME" >/dev/null || groupadd -r "$GROUPNAME"
getent passwd "$USERNAME" >/dev/null || useradd -r -g "$GROUPNAME" -M -s /sbin/nologin -c "pmm service user" "$USERNAME"
Copy link

@nikita-b nikita-b Dec 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It looks overcomplicated. I think we can use the single command:

   adduser --quiet --no-create-home  --group -c"PMM User" pmm

@nikita-b
Copy link

nikita-b commented Dec 7, 2021

What's about the defaults file?

@nikita-b
Copy link

nikita-b commented Dec 7, 2021

A couple of points:

  1. We need to register a systemd service.
  2. We need to support upgrading to a new version (even for systemd service)
  3. We need to give the user the ability to overwrite the default settings (default file)

nikita-b
nikita-b suggested changes Dec 7, 2021
View reviewed changes
.scripts/postinst Outdated
# post-installation script, and should be protected with a conditional
# so that unnecessary prompting doesn't happen if a package's
# installation fails and the `postinst' is called with `abort-upgrade',
# `abort-remove' or `abort-deconfigure'.
Copy link

Choose a reason for hiding this comment

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

Do we need this comment?

.scripts/postinst Outdated
getent group "$GROUPNAME" >/dev/null || groupadd -r "$GROUPNAME"
getent passwd "$USERNAME" >/dev/null || useradd -r -g "$GROUPNAME" -M -s /sbin/nologin -c "pmm service user" "$USERNAME"

systemctl daemon-reload > dev/null
Copy link

Choose a reason for hiding this comment

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

Debian has a lot of helper scripts. For example:
http://manpages.ubuntu.com/manpages/bionic/man1/deb-systemd-invoke.1p.html

The trick is that we need RPM and DEB packages together...

FPM can create systemd service itself but it seems that goreleaser NFPM can't...

@denisok denisok linked an issue Dec 7, 2021 that may be closed by this pull request
add deb and rpm packages #382
Closed
@denisok
Copy link
Contributor

denisok commented Dec 7, 2021

A couple of points:

1. We need to register a systemd service.

2. We need to support upgrading to a new version (even for systemd service)

3. We need to give the user the ability to overwrite the default settings (default file)

maybe we are over complicating it. Do we really need service file on thi stage? mongo exporter itself doesn't support environment or passing config file or parameter as a file (see #380).

I would probably remove service and get service later when mongodb_exporter is better parametrized, like taking user, url, pass from env.

@nikita-b nikita-b self-requested a review December 8, 2021 12:27
@denisok denisok mentioned this pull request Dec 8, 2021
Merged
@anton-bystrov anton-bystrov merged commit fff68bd into main Dec 9, 2021
@anton-bystrov anton-bystrov deleted the PMM-5598-build-deb-rpm-packages branch December 9, 2021 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denisok denisok denisok approved these changes

@nikita-b nikita-b nikita-b approved these changes

@percona-csalguero percona-csalguero Awaiting requested review from percona-csalguero

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add deb and rpm packages
4 participants
@anton-bystrov @it-percona-cla @nikita-b @denisok