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

[improve][pip] PIP-324: Alpine image #22054

Merged
merged 13 commits into from
Mar 9, 2024
Merged

Conversation

merlimat
Copy link
Contributor

PIP: PIP-324

Modifications

Changed the base Docker image from Ubuntu to Alpine.

Example Docker image: merlimat/pulsar:3.3.0-SNAPSHOT-f2a91a1

https://hub.docker.com/layers/merlimat/pulsar/3.3.0-SNAPSHOT-f2a91a1/images/sha256-723cda334135f93b366bd51920795b911e5412a55b0f4436fb81762a747dd6ec?context=explore

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: merlimat#7

@merlimat merlimat added release/important-notice The changes which are important should be mentioned in the release note ready-to-test labels Feb 14, 2024
@merlimat merlimat added this to the 3.3.0 milestone Feb 14, 2024
@merlimat merlimat self-assigned this Feb 14, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 14, 2024
@merlimat merlimat marked this pull request as draft February 14, 2024 19:37
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (beed0cf) 73.64% compared to head (f2a91a1) 73.60%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22054      +/-   ##
============================================
- Coverage     73.64%   73.60%   -0.04%     
- Complexity    32077    32568     +491     
============================================
  Files          1874     1874              
  Lines        139220   139220              
  Branches      15260    15260              
============================================
- Hits         102523   102477      -46     
- Misses        28776    28834      +58     
+ Partials       7921     7909      -12     
Flag Coverage Δ
inttests 24.70% <ø> (-0.07%) ⬇️
systests 24.32% <ø> (-0.12%) ⬇️
unittests 72.88% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 65 files with indirect coverage changes

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Great work! Few small comments. I'll review more closely tomorrow

docker/pulsar/Dockerfile Outdated Show resolved Hide resolved
docker/pulsar/Dockerfile Outdated Show resolved Hide resolved
docker/pulsar/Dockerfile Outdated Show resolved Hide resolved
docker/pulsar/Dockerfile Outdated Show resolved Hide resolved
@heesung-sn heesung-sn self-requested a review February 15, 2024 16:19
docker/pulsar/Dockerfile Outdated Show resolved Hide resolved
@merlimat
Copy link
Contributor Author

merlimat commented Mar 4, 2024

@lhotari PTAL again. I've updated the build to rely on pre-built glibc packages. The packages are going to be included in Docker image for which the Dockerfile is included here and that will be published to the apachepulsar org.

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM. Awesome job @merlimat

@merlimat merlimat marked this pull request as ready for review March 8, 2024 16:17
@merlimat merlimat merged commit 4effaa7 into apache:master Mar 9, 2024
49 of 51 checks passed
@coderzc
Copy link
Member

coderzc commented May 17, 2024

@merlimat the pulsar image based on the alpine image is unable to start normally, this issue blocks 3.3.0 release, can you take a look?

It seems that the directory cannot be created using file.mkdirs()

docker run -it \
-p 6650:6650 \
-p 8080:8080 \
merlimat/pulsar:3.3.0-SNAPSHOT-f2a91a1 \
bin/pulsar standalone
2024-05-17T03:13:58,328+0000 [main] INFO  org.apache.pulsar.broker.PulsarService - Function worker service started
2024-05-17T03:13:58,330+0000 [main] ERROR org.apache.pulsar.broker.PulsarService - Failed to start Pulsar service: Failed to create base storage directory at packages-storage
java.lang.RuntimeException: Failed to create base storage directory at packages-storage
	at org.apache.pulsar.packages.management.storage.filesystem.FileSystemPackagesStorage.initialize(FileSystemPackagesStorage.java:75) ~[org.apache.pulsar-pulsar-package-filesystem-storage-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.broker.PulsarService.startPackagesManagementService(PulsarService.java:1788) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:922) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.PulsarStandalone.start(PulsarStandalone.java:349) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.PulsarStandaloneStarter.main(PulsarStandaloneStarter.java:149) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
2024-05-17T03:13:58,336+0000 [main] ERROR org.apache.pulsar.PulsarStandaloneStarter - Failed to start pulsar service.
org.apache.pulsar.broker.PulsarServerException: java.lang.RuntimeException: Failed to create base storage directory at packages-storage
	at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:955) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.PulsarStandalone.start(PulsarStandalone.java:349) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.PulsarStandaloneStarter.main(PulsarStandaloneStarter.java:149) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
Caused by: java.lang.RuntimeException: Failed to create base storage directory at packages-storage
	at org.apache.pulsar.packages.management.storage.filesystem.FileSystemPackagesStorage.initialize(FileSystemPackagesStorage.java:75) ~[org.apache.pulsar-pulsar-package-filesystem-storage-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.broker.PulsarService.startPackagesManagementService(PulsarService.java:1788) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:922) ~[org.apache.pulsar-pulsar-broker-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
	... 2 more

@coderzc
Copy link
Member

coderzc commented May 17, 2024

I found that there is no permission to create files or directory under '/pulsar'

/pulsar $ mkdir test
mkdir: can't create directory 'test': Permission denied

/pulsar $ touch test
touch: test: Permission denied

@lhotari
Copy link
Member

lhotari commented May 17, 2024

I found that there is no permission to create files or directory under '/pulsar'

/pulsar $ mkdir test
mkdir: can't create directory 'test': Permission denied

/pulsar $ touch test
touch: test: Permission denied

This is intentional so that the image can be run with restricted permissions (read only rootfs). The writable directories should be created while building the image (for read only rootfs restricted deployments, those would have to be volumes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test release/important-notice The changes which are important should be mentioned in the release note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants