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

Gloo-Mesh Support #1165

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

rinormaloku
Copy link

Support for Gloo Mesh 2.0

@stefanprodan
Copy link
Member

@rinormaloku please signoff your commits and rebase with main instead of merge. Thanks!

@rinormaloku rinormaloku force-pushed the support-gloo-mesh branch 4 times, most recently from d4e53d9 to a2c50e7 Compare July 10, 2022 16:24
Signed-off-by: rinormaloku <rinormaloku37@gmail.com>
@rinormaloku rinormaloku changed the title [WIP] Adding support for gloo-mesh Gloo-Mesh Support Jul 10, 2022
@rinormaloku
Copy link
Author

Hi @stefanprodan,

The PR is ready for review!

@stefanprodan stefanprodan added the kind/feature Feature request label Jul 11, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1165 (5a12d7b) into main (76bac5d) will decrease coverage by 0.06%.
The diff coverage is 52.07%.

@@            Coverage Diff             @@
##             main    #1165      +/-   ##
==========================================
- Coverage   54.73%   54.67%   -0.07%     
==========================================
  Files          81       82       +1     
  Lines        6948     7117     +169     
==========================================
+ Hits         3803     3891      +88     
- Misses       2550     2616      +66     
- Partials      595      610      +15     
Impacted Files Coverage Δ
pkg/controller/scheduler_metrics.go 35.35% <0.00%> (-0.73%) ⬇️
pkg/metrics/observers/factory.go 0.00% <0.00%> (ø)
pkg/router/factory.go 0.00% <0.00%> (ø)
pkg/router/gloomesh.go 56.05% <56.05%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76bac5d...5a12d7b. Read the comment docs.

Signed-off-by: rinormaloku <rinormaloku37@gmail.com>
@rinormaloku
Copy link
Author

@stefanprodan is anything that I can help, for us to make progress with this PR?

@stefanprodan
Copy link
Member

@rinormaloku this looks good, we're working on the next patch release for Flagger then we'll review your PR. I expect this to be included in the next minor release.

@rinormaloku
Copy link
Author

Sounds good! :))

@stefanprodan
Copy link
Member

You need to add gloomesh to the e2e test suite here https://github.com/fluxcd/flagger/blob/main/.github/workflows/e2e.yaml

@rinormaloku
Copy link
Author

@stefanprodan I didn't add it to the e2e suite as Gloo Mesh requires a license and currently and it would require rotating the license frequently

@mrajkarnikar
Copy link

@stefanprodan any update on this

@stefanprodan
Copy link
Member

@rinormaloku @mrajkarnikar in the past we've rejected integrations with non OSS tools like Kong ingress. Having this integration in Flagger with no means of testing it automatically is not something that I would consider.

@rinormaloku
Copy link
Author

[...] Having this integration in Flagger with no means of testing it automatically is not something that I would consider.

If we provide a secret, would you be able to securely store it and make it available to the script as an Environment Variable?

@stefanprodan
Copy link
Member

stefanprodan commented Sep 14, 2022

Using secrets in tests means we can no longer run them on PRs. If someone breaks this integration, we can't tell until we merge the PR when it's too late. The whole purpose of testing is to validate the changes before they get merged. Also what happens when they key expires?

@mrajkarnikar
Copy link

@stefanprodan so flagger will never integrate with non oss tool? Should we be closing this PR then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants