Skip to content

Rename files on disk needed for systemd and service start.#302

Merged
mihirsoni merged 1 commit intoopensearch-project:mainfrom
shdubsh:fix_deb
Apr 26, 2021
Merged

Rename files on disk needed for systemd and service start.#302
mihirsoni merged 1 commit intoopensearch-project:mainfrom
shdubsh:fix_deb

Conversation

@shdubsh
Copy link
Copy Markdown
Contributor

@shdubsh shdubsh commented Apr 20, 2021 •
edited
Loading

Description

Files renamed to where systemd expects and runtime username
and group name changed to a supported form.

Issues Resolved

The Debian packages build ok with fpm, but installing it fails due
to an invalid username and group name (see useradd(8): CAVEATS).
Once this was fixed, the service failed to start due to parts of the startup
configuration wanting files that did not exist on disk.

Check List

  • Commits are signed per the DCO using --signoff

@shdubsh
Rename files on disk needed for systemd and service start.
6f2b817 
The Debian packages build ok with fpm, but installing it fails due
to an invalid username and group name (see useradd(8): CAVEATS).
Once this was fixed, the service failed to start due to parts of the startup
configuration wanting files that did not exist on disk.

Files renamed to where systemd expects and runtime username
and group name changed to a supported form.

Tested on Debian 10.

Signed-off-by: Cole White <cwhite@wikimedia.org>
@odfe-release-bot
Copy link
Copy Markdown

odfe-release-bot commented Apr 20, 2021

✅   DCO Check Passed 6f2b817

@mihirsoni mihirsoni added bug Something isn't working build Build related additions or modifications rename labels Apr 20, 2021
@mihirsoni mihirsoni added this to the Beta release milestone Apr 20, 2021
@mihirsoni mihirsoni linked an issue Apr 20, 2021 that may be closed by this pull request
[BUG] Unable to build and start Debian #303
Closed
mihirsoni
mihirsoni approved these changes Apr 20, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

LGTM !! Thanks for your changes. I also created issue #303

mihirsoni
mihirsoni reviewed Apr 20, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

@shdubsh Just realized the filename changes as well, any reason for renaming the file ? We are following overall naming convention and file names are following mostly _ notion instead of -

@shdubsh
Copy link
Copy Markdown
Contributor Author

shdubsh commented Apr 21, 2021 •
edited
Loading

@shdubsh Just realized the filename changes as well, any reason for renaming the file ? We are following overall naming convention and file names are following mostly _ notion instead of -

A few reasons to consider:

Underscores for system configuration files makes the compiled package inconsistent.

Prior to this patch, these files and folders are created by fpm:

  1. File /etc/default/opensearch_dashboards
  2. File /etc/init.d/opensearch_dashboards
  3. File /usr/lib/tmpfiles.d/opensearch_dashboards.conf
  4. File /etc/systemd/system/opensearch_dashboards.service
  5. Directory /etc/opensearch-dashboards
  6. Directory /usr/share/opensearch-dashboards
  7. Directory /usr/share/doc/opensearch-dashboards

In addition /etc/systemd/system/opensearch_dashboards.service is looking for the environment file at EnvironmentFile=-/etc/default/opensearch-dashboards which systemd will not find.

After this patch, these files and folders are created by fpm:

  1. File /etc/default/opensearch-dashboards
  2. File /etc/init.d/opensearch-dashboards
  3. File /usr/lib/tmpfiles.d/opensearch-dashboards.conf
  4. File /etc/systemd/system/opensearch-dashboards.service
  5. Directory /etc/opensearch-dashboards
  6. Directory /usr/share/opensearch-dashboards
  7. Directory /usr/share/doc/opensearch-dashboards

All files and directories captured in the package after this patch are consistent. (Except /etc/opensearch-dashboards/opensearch_dashboards.yml)

Renaming these files to be hyphenated also fixes systemd unable to start due to not being able to locate configuration files at /usr/share/opensearch-dashboards/config due to being unaware of OSD_PATH_CONF="/etc/opensearch_dashboards" in /etc/default/opensearch_dashboards. It was quite confusing to look and see that OpenSearch-Dashboards was configured to find its config in /etc/opensearch_dashboards, yet no folder /etc/opensearch_dashboards exists. Even if a user were to edit /etc/default/opensearch_dashboards to OSD_PATH_CONF="/etc/opensearch-dashboards", the same runtime error would be presented because systemd isn't loading /etc/default/opensearch_dashboards into the environment in the first place.

Underscores here may be contrary to Debian policy

Although it may be possible to coerce fpm to rename the Package so that opensearch-dashboards takes the underscore in /etc/ and /usr/share, this may fall foul with Debian policy. Package names (both source and binary, see Package) must consist only of lower case letters (a-z), digits (0-9), plus (+) and minus (-) signs, and periods (.). I recognize that this reason may be invalid as OpenSearch-Dashboards isn't built in a way to follow Debian policy.

Simplicity

Renaming these files is a simple way to resolve the problems in the built Debian artifact.

kavilla
kavilla approved these changes Apr 21, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

#302 (comment)

Thank you for the explanation!

@mihirsoni mihirsoni merged commit 9ebbc26 into opensearch-project:main Apr 26, 2021
kavilla pushed a commit that referenced this pull request May 21, 2021
@shdubsh @kavilla
[BUILD]Rename files on disk needed for systemd and service start. (#302)
63b9384 
The Debian packages build ok with fpm, but installing it fails due
to an invalid username and group name (see useradd(8): CAVEATS).
Once this was fixed, the service failed to start due to parts of the startup
configuration wanting files that did not exist on disk.

Files renamed to where systemd expects and runtime username
and group name changed to a supported form.

Tested on Debian 10.

Signed-off-by: Cole White <cwhite@wikimedia.org>
yubonluo pushed a commit to yubonluo/OpenSearch-Dashboards that referenced this pull request Mar 20, 2024
@SuZhou-Joe @tianleh
[Backport workspace-pr-integr]Add support for dynamic application con…
431fcdb 
…figurations (opensearch-project#5855) (opensearch-project#302)

* Add support for dynamic application configurations (opensearch-project#5855)

* Add application configuration service

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update API path name

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* implement two APIs/interfaces

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* expose get function for other plugins to use

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update interfaces

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* implement the APIs and interfaces

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add license and jsdoc

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update docs

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add more docs

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update variable name

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove unnecessary dependency

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* format readme

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* use osd version

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove debugging info

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update logging

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove lint js

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove logs

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update name style

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update function visibility and error function

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* fix unit test failures

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add unit test

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove lint file

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add more tests

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add unit tests for routes

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add remaining unit tests

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add enabled to this plugin

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update readme to mention experimental

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update change log

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* dummy commit to trigger workflow rerun

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove experimental

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add key to yml file

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove i18n

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* remove lint rc

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update comment style

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add input validation

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update unit tests

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* prevent multiple registration

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add return types

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* update readme wording

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add unit test to the plugin class about double register

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* move related ymls

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* move validation to a function

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* use trimmed versions

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* reword changelog entry

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* readability

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* add back yml change

Signed-off-by: Tianle Huang <tianleh@amazon.com>

---------

Signed-off-by: Tianle Huang <tianleh@amazon.com>

* feat: remove useless code

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

---------

Signed-off-by: Tianle Huang <tianleh@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Co-authored-by: Tianle Huang <60111637+tianleh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kavilla kavilla kavilla approved these changes

+2 more reviewers

@mihirsoni mihirsoni mihirsoni approved these changes

@dblock dblock dblock approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working build Build related additions or modifications rename

Projects

None yet

Milestone

Beta release

Development

Successfully merging this pull request may close these issues.

[BUG] Unable to build and start Debian

5 participants

@shdubsh @odfe-release-bot @dblock @mihirsoni @kavilla