-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
ci: uffizzi ephemeral environments for every PR #19415
base: main
Are you sure you want to change the base?
Conversation
5ddf015
to
4107563
Compare
@waveywaves as we discussed it in the call can you add the skip for few paths like we have in CI:
thank you |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #19415 +/- ##
==========================================
- Coverage 67.52% 65.31% -2.22%
==========================================
Files 991 1242 +251
Lines 109168 124292 +15124
Branches 2719 5443 +2724
==========================================
+ Hits 73721 81182 +7461
- Misses 31476 38767 +7291
- Partials 3971 4343 +372
Flags with carried forward coverage won't be shown. Click here to find out more. |
Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
4107563
to
2d89feb
Compare
@OrlinVasilev I have updated the workflow with the skips as you suggested. This workflow does not work on pushes and only does on pull requests so I have added the skips to the pull request bits. |
@waveywaves Does that mean if you push another commit to a PR it will not recreate the env? |
@OrlinVasilev it will recreate the env if the pushes are on the pr. What I was referring to in the earlier comment was the
push part in the above section. this is not necessary, it will be redundant is what I meant. Because the environments only get triggered on pushes to a branch from which a pull request has been opened. |
@waveywaves can you add few lines of the implementations under https://goharbor.io/docs/2.9.0/build-customize-contribute/ will be great if we have that documented and also will help out with the use case: I can help out writing this if you need me! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I think that would be a great addition to the testing capabilities that we have and will be adding great value for the new contributors to experiment
@OrlinVasilev yep let me know how I should go about it |
@waveywaves you can add new section here: https://github.com/goharbor/website/tree/main/docs/build-customize-contribute |
@OrlinVasilev I have created a basic PR to the docs goharbor/website#501. |
@Vad1mo @wy65701436 can you review please! |
@goharbor/all-maintainers can you please address your concerns(if any) here! Thank you so much! |
As the PR author shared in the community meeting, the ephemeral Harbor env is installed via harbor-helm. The concern is that some Harbor oss features need corresponding changes in the goharbor/harbor-helm repository to make it work. Thus enable this uffizzi ephemeral Harbor env in harbor oss may not cover validating all PR features. |
@zyyw we wanted to have the helm implementation so we can test all that, if we think we can revert back to the docker-compose way.. With each new PR we will be testing against/with the current helm chart, which I don't see to be an issue. If we want to have a special helm chart version maybe @waveywaves can comment on this if there is a possibility to make that flexible. |
@OrlinVasilev I have a different opinion on this though. As mentioned above, if a feature implemented in one PR within goharbor/harbor repository and it requires corresponding changes in goharbor/harbor-helm as well to make it work for those deployment via harbor-helm. Then, uffizzi, deploying harbor via harbor-helm, does not actually test the PR changes on goharbor/harbor. Because this PR is not merged into the main branch of goharbor/harbor yet. One concrete example would be this: |
@zyyw Agree with you, but that is a corner case, right? What's your process for testing cases like this so maybe we can have uffizzi implement it :) |
@zyyw That is correct, considering the harbor helm chart naturally won't be able to keep up with the developments in harbor as is the case with any projects and their deployments, you won't be able to test the latest harbor features with your existing helm chart. But you would be able to test developments in existing harbor features with the available helm chart deployment, and I am sure that would benefit developers working on existing features quite a bit. On that note, we are working on multi PR environments through which you should be able to test changes from multiple PRs in one cluster. @OrlinVasilev you are right this is an edge case but, Uffizzi should still be very helpful for developers working on existing features and we can work on extended solutions which can help drive productivity here. |
@waveywaves thanks for your response! Indeed I think even a bit limited we still can benefit quite extensively by be able to test with Uffizzi! |
@zyyw can you please share your workflow , thank you! |
IMO, if Uffizzi can manage to install harbor via docker-compose, then it would be beneficial to the goharbor/harbor project to test/validate PRs within goharbor/harbor project. Otherwise, Uffizzi's currently implementation, which deploys Harbor via harbor-helm, actually tests/validates PRs within goharbor/harbor-helm project. As Uffizzi deploys harbor via harbor-helm, and harbor-helm images use @waveywaves @OrlinVasilev please correct me if I misunderstand anything. Thanks |
@waveywaves how hard is it to rework it ? |
Following the community meeting, it appears that Uffizzi can exclusively deploy Harbor using the Helm chart. However, within the Harbor core, there's a need to establish a process for synchronizing code changes from the current pull request to Helm. Otherwise, given we merge this, it still cannot get the environment with the latest code change. @waveywaves this should be an blocker that needs to be figured out. |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main. |
Thank you for contributing to Harbor!
Comprehensive Summary of your change
This PR adds a ephemeral environment solution for harbor to support creating virtual clusters for every pull request opened in this repo. After this PR is merged, all subsequent PRs created will each have a virtual cluster spun up and a harbor deployments done in the cluster. The harbor services deployed would be built from the changes in the pull request.
The uffizzi cluster lifecycle and ingress creation
When the uffizzi cluster creation workflow succeeds in the PR, a comment, similar to the one below will be posted. The comment will also contains a link to to the ingress for the instances of harbor services running in the cluster
waveywaves#1 (comment)
https://my-release-harbor-ingress-default-pr-1-c916.uclusters.app.uffizzi.com/
The above is an example of what the url to the harbor instance will look like. Here,
my-release-harbor-ingress
is the ingress name,default
is the namespace where harbor is deployedpr-1
is the cluster name which can be accessed by the user.Once the PR is closed, the cluster and therefore the resources within will be purged. The ingress will also not work thereafter.
Creation of Uffizzi Clusters is quick and easy. The clusters will be accessible to the developer opening the PR and the admins of the project. These will improve collaboration through a singular ephemeral environment which can be shared between developers and other stakeholders.
Issue being fixed
Fixes #18006
Please indicate you've done the following:
cc @OrlinVasilev