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

feat: add profile to launch harbor as external registry #862

Merged
merged 28 commits into from
May 21, 2024
Merged

feat: add profile to launch harbor as external registry #862

merged 28 commits into from
May 21, 2024

Conversation

guilhem-barthes
Copy link
Contributor

@guilhem-barthes guilhem-barthes commented Mar 25, 2024 •
edited
Loading

Description

Add a new skaffold profile to use a Harbor registry instead of the Docker one

Also fixes FL-1477 & FL-1479, which was needed for creating the skaffold profile

Companion PR

How has this been tested?

Checklist

  • changelog was updated with notable changes
  • documentation was updated

@guilhem-barthes guilhem-barthes changed the title Feat/add profile to launch harbor as external registry+ feat: add profile to launch harbor as external registry+ Mar 26, 2024
@guilhem-barthes guilhem-barthes force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch from 2e22c07 to 2771b23 Compare March 26, 2024 14:31
@guilhem-barthes guilhem-barthes changed the title feat: add profile to launch harbor as external registry+ feat: add profile to launch harbor as external registry Mar 26, 2024
@linear Linear
Copy link

linear bot commented Mar 26, 2024

FL-1477 Make the docker registry customisable

@guilhem-barthes guilhem-barthes force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch from 15a7a6c to a0324d9 Compare March 26, 2024 14:55
@guilhem-barthes guilhem-barthes marked this pull request as ready for review March 27, 2024 12:30
@guilhem-barthes guilhem-barthes requested a review from a team as a code owner March 27, 2024 12:30
@guilhem-barthes guilhem-barthes force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch 2 times, most recently from a690665 to 08cb7ef Compare April 15, 2024 06:52
SdgJlbl
SdgJlbl approved these changes Apr 16, 2024
View reviewed changes
backend/builder/docker.py Show resolved Hide resolved
@guilhem-barthes guilhem-barthes force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch from 08cb7ef to 136a748 Compare April 16, 2024 07:36
@guilhem-barthes guilhem-barthes mentioned this pull request Apr 16, 2024
Merged
@linear Linear
Copy link

linear bot commented Apr 16, 2024

FL-1479 Deploy Harbor locally with skaffold

@guilhem-barthes guilhem-barthes requested a review from SdgJlbl April 16, 2024 07:48
@guilhem-barthes guilhem-barthes force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch from 136a748 to 4e9d4c8 Compare April 18, 2024 07:06
@SdgJlbl SdgJlbl force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch from 43889c4 to 3c70c7d Compare April 22, 2024 16:17
@SdgJlbl
Copy link
Contributor

SdgJlbl commented Apr 22, 2024

/e2e

@Owlfred
Copy link

Owlfred commented Apr 22, 2024 •
edited
Loading

End to end tests: ✔️ SUCCESS

Nailed it!

@SdgJlbl SdgJlbl force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch 3 times, most recently from d1b6eb8 to d085b55 Compare April 24, 2024 12:41
@SdgJlbl
Copy link
Contributor

SdgJlbl commented Apr 24, 2024

/e2e --tests=doc

@Owlfred
Copy link

Owlfred commented Apr 24, 2024 •
edited
Loading

End to end tests: ✔️ SUCCESS

thbcmlowk
thbcmlowk reviewed Apr 24, 2024
View reviewed changes
charts/substra-backend/CHANGELOG.md Outdated
Comment on lines 5 to 9
## [26.4.0] - 2024-04-23

### Added

- Provides `.Values.kaniko.dockerConfigSecretName` to the executor (#862)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to update both the charts CHANGELOG and the towncrier changes folder?

Copy link
Contributor

Choose a reason for hiding this comment

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

We're in CD, so the changelog needs to be generated at each "release" (PR).
You can either update the changelog directly, or use towncrier and generate the new changelog.
(towncrier is more useful when it's a bot updating the changelog)

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean:

  1. Why do we update towncrier-related files if we still need to perform the old fashion manual updates?
  2. Why don't we leverage the use of towncrier in substra-gha-workflows so we don't bother with manual updates anymore? 😃

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally just make the update for the charts changelog by hand for now. Setting up towncrier just saved like 50 lines of sed on the automation script.
That being said, I'm totally for leveraging the towncrier fragments in the gha workflow so that the changelog is generated automatically :)
(I didn't even know where the GHA were defined before today 😅 )

Copy link
Contributor

Choose a reason for hiding this comment

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

creating a card for that ⬆️

@SdgJlbl SdgJlbl force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch from d085b55 to a38af59 Compare April 29, 2024 14:05
@guilhem-barthes @SdgJlbl
feat: add docker-config secret
f9f191e 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
feat: add harbor as repository
dd3586a 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
fix: remove useless metadata
d5b3338 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
guilhem-barthes and others added 22 commits May 17, 2024 16:41
@guilhem-barthes @SdgJlbl
feat: being able to push manually substra-tools
4a7f400 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
wip: most example works
3ff50ac 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
feat: add credentials to DXF
1a8ff4a 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
chore: factorise get_registry_auth
d6169bb 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
fix: use schemaVersion version 2 to get entrypoint
11ee94c 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
fix: remove raise in get_registry_auth
ced1b3a 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
feat: add docker-secret to kubernetes
ba09f81 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
wip: registries config
3526b1d 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
chore: remove temporary values
cd57102 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
feat: add TLS verification
70b2b49 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
docs: create change fragments
363f38b 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
fix: commit changes with new profiles
d98c087 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
feat: generate certificates for registry
3b2b808 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
chore: create namespace
eb410b3 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes @SdgJlbl
wip: add blanket network policy for skaffold opening all ports
2954849 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@SdgJlbl
chore: rebase fixes + linter
68bb899 
Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>
@SdgJlbl
chore: generate charts changelog
5ec695d 
Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>
@SdgJlbl
chore:fix tests
5a1e615 
Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>
@guilhem-barthes @SdgJlbl
feat: add network policy for harbor and distant registries
cb56bed 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@SdgJlbl
chore: remove network policies
1be6f5c 
Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>
@SdgJlbl
chore: drop skaffold profile for harbor
671496f 
Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>
@SdgJlbl
chore: bump chart version
8015f7b 
Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>
@SdgJlbl SdgJlbl force-pushed the feat/add-profile-to-launch-harbor-as-external-registry+ branch from ab04540 to 8015f7b Compare May 17, 2024 15:21
thbcmlowk
thbcmlowk approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@thbcmlowk thbcmlowk left a comment

Choose a reason for hiding this comment

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

Thank you! I see some YAML files in the skaffold examples that does not seem necessary to allow plugging an external Harbor but still LGTM.

@guilhem-barthes
fix: secrets
6506778 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
fix: remove cookies when communicating with Harbor
eac630e 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@SdgJlbl SdgJlbl merged commit d88d3d1 into main May 21, 2024
10 checks passed
@SdgJlbl SdgJlbl deleted the feat/add-profile-to-launch-harbor-as-external-registry+ branch May 21, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SdgJlbl SdgJlbl SdgJlbl approved these changes

@thbcmlowk thbcmlowk thbcmlowk approved these changes

Assignees
No one assigned
Labels
compute-engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@guilhem-barthes @SdgJlbl @Owlfred @thbcmlowk