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

♻️ Migrate to docker stack config #5523

Merged
merged 27 commits into from
Apr 3, 2024

Conversation

mrnicegyu11
Copy link
Member

@mrnicegyu11 mrnicegyu11 commented Mar 20, 2024

What do these changes do?

  • Migrates our custom tooling around docker config in favor of using docker stack config, as recommended by docker
  • Double defaults are not supported by docker stack compose and results in
    image
  • Fixes env-vars that caused troubles in this change
  • Adds a pre-commit hook that checks for problematic env-vars

Related PRs

ITISFoundation/osparc-ops-environments#613
https://git.speag.com/oSparc/osparc-ops-deployment-configuration/-/merge_requests/501#19f9eb849fe8623b5079612f75102c63d695b03e

How to test

Dev-ops checklist

@mrnicegyu11 mrnicegyu11 added bug buggy, it does not work as expected High Priority a totally crucial bug/feature to be fixed asap a:apiserver api-server service python labels Mar 20, 2024
@mrnicegyu11 mrnicegyu11 added this to the Schoggilebe milestone Mar 20, 2024
@mrnicegyu11 mrnicegyu11 self-assigned this Mar 20, 2024
Copy link

codecov bot commented Mar 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.4%. Comparing base (cafbf96) to head (bc93a72).
Report is 79 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5523      +/-   ##
=========================================
+ Coverage    84.5%   86.4%    +1.9%     
=========================================
  Files          10     935     +925     
  Lines         214   42282   +42068     
  Branches       25     788     +763     
=========================================
+ Hits          181   36567   +36386     
- Misses         23    5534    +5511     
- Partials       10     181     +171     
Flag Coverage Δ
integrationtests 65.1% <ø> (?)
unittests 85.0% <ø> (+0.5%) ⬆️

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

Files Coverage Δ
.../simcore_service_webserver/application_settings.py 98.2% <ø> (ø)

... and 940 files with indirect coverage changes

@mrnicegyu11 mrnicegyu11 requested a review from pcrespov March 20, 2024 11:40
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

thx. Please check if there are more out there. I found at least one more.
I would also check in the osparc-config to be safe

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

I guess this is not needed anymore right?

@mrnicegyu11 mrnicegyu11 changed the title Remove double defaults in env-var of api-server Migrate to docker stack deploy Mar 27, 2024
@mrnicegyu11 mrnicegyu11 changed the title Migrate to docker stack deploy Migrate to docker stack config Mar 27, 2024
@mrnicegyu11 mrnicegyu11 changed the title Migrate to docker stack config ♻️ Migrate to docker stack config Mar 27, 2024
sanderegg and others added 8 commits April 2, 2024 10:32
* revert changes

* have docker stack config for clarity

* replaced docker compose config by docker stack config

* replaced docker compose config by docker stack config

* fix issue

* add a note

* delete file

* fix note
* remove sed stuff

* remove sed

* go for info

* missing variable

* dask stack config does not support nested ENV definitions
mrnicegyu11 pushed a commit to mrnicegyu11/osparc-ops-environments that referenced this pull request Apr 2, 2024
Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Nice bash scripting skills! I bet that was a lot of fun 😉

services/docker-compose.yml Show resolved Hide resolved
Copy link
Contributor

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

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

Thanks!

@mrnicegyu11 mrnicegyu11 enabled auto-merge (squash) April 3, 2024 09:27
Copy link

sonarcloud bot commented Apr 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.4% Duplication on New Code

See analysis details on SonarCloud

@mrnicegyu11 mrnicegyu11 disabled auto-merge April 3, 2024 15:01
@mrnicegyu11 mrnicegyu11 merged commit 7f25edf into ITISFoundation:master Apr 3, 2024
56 checks passed
@mrnicegyu11
Copy link
Member Author

mrnicegyu11 commented Apr 3, 2024

test passed, force merged without @sanderegg approval since he gave it yesterday and today he is off

@mrnicegyu11 mrnicegyu11 deleted the fix/apiserverCDenvvar branch April 3, 2024 15:02
mrnicegyu11 added a commit to ITISFoundation/osparc-ops-environments that referenced this pull request Apr 8, 2024
* Revert "Revert "Try to fix CD - 3""

This reverts commit 945b576.

* Rename obscure bash scripts

* Remove stale comment

* add comment

* Remove references to env-devel

* Fix devenv venv: install typer

* Pull docker-stack-config.bash from github.com/ITISFoundation/osparc-simcore/pull/5523

* Fix bug: Remove unused secrets

* Handle docker-in-docker situations and docker secrets handling in gitlab Ci runners

* Fix bug spotted by @YuryHrytsuk

---------

Co-authored-by: kaiser <[email protected]>
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Apr 26, 2024
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:apiserver api-server service bug buggy, it does not work as expected High Priority a totally crucial bug/feature to be fixed asap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants