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.

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

dockerfiles: (re)name suse Dockerfile as expected. #999

Merged
merged 1 commit into from Apr 19, 2023

Conversation

klihub
Copy link
Contributor

@klihub klihub commented Apr 17, 2023

Building native distro packages with 'make packages' expects the distro-specific Dockerfile to have the '-opensuse-leap-$VERSION' suffix when building on opensuse, since this is how the distro names itself in /etc/os-release. (Re)name the Dockerfile to be as expected.

@klihub klihub temporarily deployed to dev April 17, 2023 07:59 — with GitHub Actions Inactive
@klihub klihub temporarily deployed to dev April 17, 2023 07:59 — with GitHub Actions Inactive
klihub added a commit to klihub/cri-resource-manager that referenced this pull request Apr 17, 2023
[cherry-picked intel#999/a54f3165]

Building native distro packages with 'make packages'
expects the distro-specific Dockerfile to have the
'-opensuse-leap-$VERSION' suffix  when building on
opensuse, since this is how the distro names itself
in /etc/os-release. (Re)name the Dockerfile to be
as expected.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@marquiz
Copy link
Contributor

marquiz commented Apr 17, 2023

Sorry, does this fix some issue? Seems to work fine for me, e.g.make cross-rpm.suse-15.4 generated an rpm as expected

marquiz
marquiz previously approved these changes Apr 17, 2023
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2023

Codecov Report

Merging #999 (a15dc4a) into master (fd31c37) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #999   +/-   ##
=======================================
  Coverage   31.86%   31.86%           
=======================================
  Files          65       65           
  Lines        9835     9835           
=======================================
  Hits         3134     3134           
  Misses       6411     6411           
  Partials      290      290           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@klihub
Copy link
Contributor Author

klihub commented Apr 17, 2023

Sorry, does this fix some issue? Seems to work fine for me, e.g.make cross-rpm.suse-15.4 generated an rpm as expected

Yes. Clone the repo on any machine running opensuse and try 'make packages'. It fails without this commit and succeeds with this one present in the tree.

@klihub
Copy link
Contributor Author

klihub commented Apr 17, 2023

Sorry, does this fix some issue? Seems to work fine for me, e.g.make cross-rpm.suse-15.4 generated an rpm as expected

Yes. Clone the repo on any machine running opensuse and try 'make packages'. It fails without this commit and succeeds with this one present in the tree.

And the reason is that the distro calls itself opensuse in /etc/os-release.

jukkar
jukkar previously approved these changes Apr 17, 2023
@klihub
Copy link
Contributor Author

klihub commented Apr 17, 2023

Hmm... but you are right that there might be a better/easier fix for that. And I think this might break some (packaging) tests without further changes.

@klihub klihub dismissed stale reviews from jukkar and marquiz via 633b1a4 April 17, 2023 11:09
@klihub klihub force-pushed the fixes/opensuse-dockerfile-name branch from a54f316 to 633b1a4 Compare April 17, 2023 11:09
@klihub klihub temporarily deployed to dev April 17, 2023 11:10 — with GitHub Actions Inactive
@klihub klihub temporarily deployed to dev April 17, 2023 11:10 — with GitHub Actions Inactive
klihub added a commit to klihub/cri-resource-manager that referenced this pull request Apr 17, 2023
[cherry-picked intel#999/633b1a4e]

Building native distro packages with 'make packages'
expects the distro-specific Dockerfile to have the
'-opensuse-leap-$VERSION' suffix  when building on
opensuse, since this is how the distro names itself
in /etc/os-release. (Re)name the Dockerfile to be
as expected.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Building native distro packages with 'make packages'
expects the distro-specific Dockerfile to have the
'-opensuse-leap-$VERSION' suffix  when building on
opensuse, since this is how the distro names itself
in /etc/os-release. (Re)name the Dockerfile to be
as expected.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub klihub force-pushed the fixes/opensuse-dockerfile-name branch from 633b1a4 to a15dc4a Compare April 17, 2023 11:14
@klihub klihub temporarily deployed to dev April 17, 2023 11:14 — with GitHub Actions Inactive
@klihub klihub temporarily deployed to dev April 17, 2023 11:14 — with GitHub Actions Inactive
klihub added a commit to klihub/cri-resource-manager that referenced this pull request Apr 17, 2023
[cherry-picked intel#999/a15dc4a8]

Building native distro packages with 'make packages'
expects the distro-specific Dockerfile to have the
'-opensuse-leap-$VERSION' suffix  when building on
opensuse, since this is how the distro names itself
in /etc/os-release. (Re)name the Dockerfile to be
as expected.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub
Copy link
Contributor Author

klihub commented Apr 17, 2023

Hmm... but you are right that there might be a better/easier fix for that. And I think this might break some (packaging) tests without further changes.

Updated commits with 2 related fixes/adjustments, one in the docs and another in a packaging test case.

Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

LGTM

@marquiz marquiz merged commit 01c6ef9 into intel:master Apr 19, 2023
4 checks passed
@marquiz marquiz mentioned this pull request Jan 8, 2024
18 tasks
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.

None yet

4 participants