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

Restruct pkg/ingress #1204

Merged
merged 28 commits into from
Aug 10, 2022
Merged

Conversation

lingsamuel
Copy link
Member

@lingsamuel lingsamuel commented Aug 2, 2022

Type of change:

  • Refactor

What this PR does / why we need it:

Part of #1058

Submit PR for CI, I will update the changes later

This is a huge PR, so I will describe the changes and ideas in detail.

  • This PR is expected to be merged in 1.16, so I delete the apisix-route-version option. If this option should be kept for more versions, I will restore it.

  • This PR splits pkg/ingress into multiple providers, including apisix, ingress and k8s, and gateway-api was also split in a previous PR.

    • Controllers are no longer dependent on the parent controller. If a controller needs data held by other controllers, the data should be exposed by their provider interface. (See NamespaceProvider and PodProvider)
    • There are separate translators for "apisix" and "ingress". Since this PR doesn't change any detailed translation logic, ingress translator depends on apisix translator for now.
    • The CRD controllers handle own status records instead of using the shared type-switch function.
    • Some shared variables, such as MetricsCollector, KubeClient, and config, are embedded by commonConfig.
  • If the E2E_ENV is not ci, replace ingress-controller deployment yaml imagePullPolicy to Always.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
@lingsamuel lingsamuel changed the title Restruct pkgingress Restruct pkg/ingress Aug 2, 2022
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #1204 (5b89467) into master (cccad72) will increase coverage by 13.09%.
The diff coverage is 31.47%.

@@             Coverage Diff             @@
##           master    #1204       +/-   ##
===========================================
+ Coverage   29.55%   42.64%   +13.09%     
===========================================
  Files          81       73        -8     
  Lines       10166     6462     -3704     
===========================================
- Hits         3005     2756      -249     
+ Misses       6834     3409     -3425     
+ Partials      327      297       -30     
Impacted Files Coverage Δ
cmd/ingress/ingress.go 78.64% <ø> (-0.41%) ⬇️
pkg/config/config.go 65.00% <ø> (-0.35%) ⬇️
...viders/apisix/translation/apisix_cluster_config.go 50.00% <ø> (ø)
...kg/providers/apisix/translation/apisix_consumer.go 67.74% <ø> (ø)
pkg/providers/apisix/translation/apisix_ssl.go 0.00% <0.00%> (ø)
pkg/providers/apisix/translation/translator.go 0.00% <0.00%> (ø)
pkg/providers/gateway/translation/gateway.go 0.00% <ø> (ø)
.../providers/gateway/translation/gateway_tlsroute.go 0.00% <0.00%> (ø)
pkg/providers/gateway/translation/translator.go 0.00% <ø> (ø)
pkg/providers/ingress/provider.go 0.00% <0.00%> (ø)
... and 40 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@tao12345666333 tao12345666333 added this to the v1.6.0 milestone Aug 2, 2022
Signed-off-by: Ling Samuel <lingsamuelgrace@gmail.com>
@tao12345666333 tao12345666333 self-assigned this Aug 5, 2022
Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

LGTM

let's move forward

Init(ctx context.Context) error
ResourceSync()

GetSslFromSecretKey(string) *sync.Map
Copy link
Member

Choose a reason for hiding this comment

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

just a question, why do we put it here, is it a generic method?

@tao12345666333 tao12345666333 merged commit d32c728 into apache:master Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants