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

Allow simultaneous initialization of common html volume #1728

Merged
merged 1 commit into from
Apr 22, 2022
Merged

Allow simultaneous initialization of common html volume #1728

merged 1 commit into from
Apr 22, 2022

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Apr 11, 2022 •
edited
Loading

Context

  • Have multiple docker containers running behind a load balancer
  • Share the html files directory (it helps for various reasons, one of it being the htaccess update needed in all containers otherwise)
  • Start multiple containers at the same time

Issue

  • Rsync is complaining because files gets created/deleted from multiple sources simultaneously

Solution

  • Add a lock file to make sure only one container is allowed to init the shared html folder at the same time
  • Lock file is shared for all containers within /var/www/html
  • If html is not shared, then this will not be an issue and the lock file will be useless

@skjnldsv skjnldsv self-assigned this Apr 11, 2022
@skjnldsv skjnldsv force-pushed the feat/simultaneaous-html-init branch from d02905c to d9af85e Compare April 11, 2022 12:21
@J0WI
Copy link
Contributor

J0WI commented Apr 12, 2022

There's a syntax error, all tests are failing.

@skjnldsv skjnldsv force-pushed the feat/simultaneaous-html-init branch from d9af85e to f0d74cd Compare April 13, 2022 08:00
@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv skjnldsv force-pushed the feat/simultaneaous-html-init branch from f0d74cd to ec0c741 Compare April 13, 2022 08:28
@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv skjnldsv force-pushed the feat/simultaneaous-html-init branch 2 times, most recently from dcccbc4 to bd70f0e Compare April 13, 2022 08:36
@skjnldsv

This comment was marked as resolved.

Sign in to view
@J0WI

This comment was marked as resolved.

Sign in to view
@skjnldsv skjnldsv force-pushed the feat/simultaneaous-html-init branch from bd70f0e to 98c4e1e Compare April 13, 2022 08:41
@skjnldsv
Copy link
Member Author

@J0WI done :)

@pierreozoux
Copy link
Member

pierreozoux commented Apr 13, 2022 •
edited
Loading

I'd like to review, but I changed my mind about putting the html in the container a long time ago.
I think it was a mistake.

On kubernetes we don't use it anymore and are really happy :)

I tried here without much success 🙃

@skjnldsv
Copy link
Member Author

I'd like to review, but I changed my mind about putting the html in the container a long time ago.

I agree with you! This is also what I'm going forward with. There are a few issues related to stateless containers (like updating htaccess), but this is more or less a sanity check if people wanna do it.
There are multiple ways of addressing scalability :)

@skjnldsv
Copy link
Member Author

/rebase

@skjnldsv skjnldsv force-pushed the feat/simultaneaous-html-init branch from 98c4e1e to 35b8ebf Compare April 22, 2022 08:05
@skjnldsv skjnldsv merged commit b842cb3 into master Apr 22, 2022
@skjnldsv skjnldsv deleted the feat/simultaneaous-html-init branch April 22, 2022 08:22
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Apr 26, 2022
@docker-library-bot
Update nextcloud
55bfcf7 
Changes:

- https///github.com/nextcloud/docker/commit/00c5180: Merge pull request https///github.com/nextcloud/docker/pull/1737 from J0WI/readme-typos
- https///github.com/nextcloud/docker/commit/a497f03: Runs update.sh
- https///github.com/nextcloud/docker/commit/f511ef2: Minor typos
- https///github.com/nextcloud/docker/commit/b842cb3: Merge pull request https///github.com/nextcloud/docker/pull/1728 from nextcloud/feat/simultaneaous-html-init
- https///github.com/nextcloud/docker/commit/35b8ebf: Allow simultaneous initialization of common html volume
- https///github.com/nextcloud/docker/commit/e76b7ca: Merge pull request https///github.com/nextcloud/docker/pull/1732 from nextcloud/revert-1684-feature/healthcheck
- https///github.com/nextcloud/docker/commit/ca1e773: Revert "Add healthcheck"
- https///github.com/nextcloud/docker/commit/f9d8052: Merge pull request https///github.com/nextcloud/docker/pull/1684 from TheLastProject/feature/healthcheck
- https///github.com/nextcloud/docker/commit/4bb8d00: Create command-rebase.yml
- https///github.com/nextcloud/docker/commit/1bd3a50: Merge pull request https///github.com/nextcloud/docker/pull/1730 from nextcloud/add/summary-mandatory-step
- https///github.com/nextcloud/docker/commit/8355426: Add healthcheck
- https///github.com/nextcloud/docker/commit/58273cd: Merge pull request https///github.com/nextcloud/docker/pull/1698 from t3easy/hide-nginx-infos
- https///github.com/nextcloud/docker/commit/2753dad: Add mandatory summary step for branches protection
- https///github.com/nextcloud/docker/commit/da935d2: Runs update.sh
- https///github.com/nextcloud/docker/commit/e36ca5f: Fix regex for pecl prereleases (https///github.com/nextcloud/docker/pull/1725)
- https///github.com/nextcloud/docker/commit/71f4a94: update.sh: Remove unused paths (https///github.com/nextcloud/docker/pull/1723)
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Apr 26, 2022
@docker-library-bot
Update nextcloud
1804b6c 
Changes:

- https///github.com/nextcloud/docker/commit/3f42156: 23.0.4 stable (https///github.com/nextcloud/docker/pull/1738)
- https///github.com/nextcloud/docker/commit/00c5180: Merge pull request https///github.com/nextcloud/docker/pull/1737 from J0WI/readme-typos
- https///github.com/nextcloud/docker/commit/a497f03: Runs update.sh
- https///github.com/nextcloud/docker/commit/f511ef2: Minor typos
- https///github.com/nextcloud/docker/commit/b842cb3: Merge pull request https///github.com/nextcloud/docker/pull/1728 from nextcloud/feat/simultaneaous-html-init
- https///github.com/nextcloud/docker/commit/35b8ebf: Allow simultaneous initialization of common html volume
- https///github.com/nextcloud/docker/commit/e76b7ca: Merge pull request https///github.com/nextcloud/docker/pull/1732 from nextcloud/revert-1684-feature/healthcheck
- https///github.com/nextcloud/docker/commit/ca1e773: Revert "Add healthcheck"
- https///github.com/nextcloud/docker/commit/f9d8052: Merge pull request https///github.com/nextcloud/docker/pull/1684 from TheLastProject/feature/healthcheck
- https///github.com/nextcloud/docker/commit/4bb8d00: Create command-rebase.yml
- https///github.com/nextcloud/docker/commit/1bd3a50: Merge pull request https///github.com/nextcloud/docker/pull/1730 from nextcloud/add/summary-mandatory-step
- https///github.com/nextcloud/docker/commit/8355426: Add healthcheck
- https///github.com/nextcloud/docker/commit/58273cd: Merge pull request https///github.com/nextcloud/docker/pull/1698 from t3easy/hide-nginx-infos
- https///github.com/nextcloud/docker/commit/2753dad: Add mandatory summary step for branches protection
- https///github.com/nextcloud/docker/commit/da935d2: Runs update.sh
- https///github.com/nextcloud/docker/commit/e36ca5f: Fix regex for pecl prereleases (https///github.com/nextcloud/docker/pull/1725)
- https///github.com/nextcloud/docker/commit/71f4a94: update.sh: Remove unused paths (https///github.com/nextcloud/docker/pull/1723)
GuyPaddock pushed a commit to Inveniem/nextcloud-azure-aks that referenced this pull request Sep 11, 2022
Update to Nextcloud 23.0.9 + Add Support for Initialization Locking
ae72ff4 
This toggles on the new locking support added to 23.0.4 in:
nextcloud/docker#1728

It can be disabled with NEXTCLOUD_INIT_LOCK being set to `false`, but
we default it on in our image since we support multiple pods by default.
GuyPaddock pushed a commit to Inveniem/nextcloud-azure-aks that referenced this pull request Sep 11, 2022
Update to Nextcloud 23.0.9 + Add Support for Initialization Locking
df0b52a 
This toggles on the new locking support added to 23.0.4 in:
nextcloud/docker#1728

It can be disabled with NEXTCLOUD_INIT_LOCK being set to `false`, but
we default it on in our image since we support multiple pods by default.
GuyPaddock pushed a commit to Inveniem/nextcloud-azure-aks that referenced this pull request Sep 13, 2022
Update to Nextcloud 23.0.9 + Add Support for Initialization Locking
ba1b2b5 
This toggles on the new locking support added to 23.0.4 in:
nextcloud/docker#1728

It can be disabled with NEXTCLOUD_INIT_LOCK being set to `false`, but
we default it on in our image since we support multiple pods by default.
@joshtrichards joshtrichards mentioned this pull request Jul 19, 2023
Open
@creekorful creekorful mentioned this pull request Oct 2, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J0WI J0WI J0WI approved these changes

@tilosp tilosp Awaiting requested review from tilosp

@pierreozoux pierreozoux Awaiting requested review from pierreozoux

Assignees

@skjnldsv skjnldsv

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@skjnldsv @J0WI @pierreozoux