Skip to content

Implements Config API#146

Merged
danehans merged 7 commits into
envoyproxy:mainfrom
danehans:cfg_api_impl
Jul 12, 2022
Merged

Implements Config API#146
danehans merged 7 commits into
envoyproxy:mainfrom
danehans:cfg_api_impl

Conversation

@danehans
Copy link
Copy Markdown
Contributor

@danehans danehans commented Jul 7, 2022

  • Adds server command to serve Envoy Gateway.
  • Adds logr logging framework and uses zapr as the default logging implementation.
  • Adds initial Kubernetes provider implementation.
  • Adds make target to run EG locally against a k8s cluster.

Signed-off-by: danehans daneyonhansen@gmail.com

@danehans danehans requested a review from a team as a code owner July 7, 2022 21:23
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 7, 2022 •
edited
Loading

Codecov Report

Merging #146 (de93191) into main (0fe5e38) will decrease coverage by 1.17%.
The diff coverage is 51.17%.

@@            Coverage Diff             @@
##             main     #146      +/-   ##
==========================================
- Coverage   54.25%   53.08%   -1.18%     
==========================================
  Files           6       12       +6     
  Lines         317      486     +169     
==========================================
+ Hits          172      258      +86     
- Misses        137      210      +73     
- Partials        8       18      +10
Impacted Files Coverage Δ
pkg/envoygateway/config/config.go 0.00% <0.00%> (ø)
pkg/provider/provider.go 23.07% <23.07%> (ø)
internal/cmd/server.go 24.32% <24.32%> (ø)
pkg/envoygateway/config/decoder.go 70.00% <70.00%> (ø)
pkg/provider/kubernetes/gatewayclass.go 72.34% <72.34%> (ø)
pkg/provider/kubernetes/kubernetes.go 73.33% <73.33%> (ø)
internal/cmd/root.go 100.00% <100.00%> (ø)

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 0fe5e38...de93191. Read the comment docs.

kflynn
kflynn reviewed Jul 7, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@kflynn kflynn left a comment

Choose a reason for hiding this comment

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

Per the community meeting today, maybe crank up the code coverage – but yeah, cool to see the framework in place!

arkodg
arkodg reviewed Jul 8, 2022
View reviewed changes
Comment thread hack/run-kube-local.sh Outdated
arkodg
arkodg reviewed Jul 8, 2022
View reviewed changes
Comment thread internal/cmd/server.go Outdated
@arkodg arkodg mentioned this pull request Jul 8, 2022
Closed
arkodg
arkodg reviewed Jul 8, 2022
View reviewed changes
Comment thread internal/cmd/server.go Outdated
@danehans
Implements Config API
45bbb6d 
Signed-off-by: danehans <daneyonhansen@gmail.com>
@danehans
Resolves Arko Feedback
984958b 
Signed-off-by: danehans <daneyonhansen@gmail.com>
danehans added 3 commits July 11, 2022 15:55
@danehans
Adds tests
86799dd 
Signed-off-by: danehans <daneyonhansen@gmail.com>
@danehans
Bumps controller-runtime dep
bd1fac7 
Signed-off-by: danehans <daneyonhansen@gmail.com>
@danehans
Adds kube provider deps to github actions
a1b96b3 
Signed-off-by: danehans <daneyonhansen@gmail.com>
youngnick
youngnick previously approved these changes Jul 12, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

LGTM, nice work.

@danehans
Adds client param to kube provider.New() for testing
386f0cc 
Signed-off-by: danehans <daneyonhansen@gmail.com>
arkodg
arkodg reviewed Jul 12, 2022
View reviewed changes
Comment thread hack/actions/install-kubernetes-toolchain.sh Outdated
arkodg
arkodg reviewed Jul 12, 2022
View reviewed changes
Comment thread .github/workflows/build_and_test.yaml Outdated
uses: actions/setup-go@v3
with:
go-version: ${{ env.GO_VERSION }}
- name: deps
Copy link
Copy Markdown
Contributor

@arkodg arkodg Jul 12, 2022

Choose a reason for hiding this comment

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

we should convert this into a make target
ptal at https://github.com/envoyproxy/gateway/pull/146/files#r919220859

Copy link
Copy Markdown
Contributor Author

@danehans danehans Jul 12, 2022

Choose a reason for hiding this comment

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

@arkodg commit de93191 removes this target. Instead, the existing go.test.coverage target calls the envtest target to install the setup-envtest bin and sets the KUBEBUILDER_ASSETS env var used by the kube provider integration tests.

Copy link
Copy Markdown
Contributor

@arkodg arkodg Jul 12, 2022

Choose a reason for hiding this comment

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

ah fine for now, but we should consider adding a separate make target for kube integration tests in the future

@danehans
Removes GHA deps install script
de93191 
Signed-off-by: danehans <daneyonhansen@gmail.com>
arkodg
arkodg approved these changes Jul 12, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@danehans
Copy link
Copy Markdown
Contributor Author

danehans commented Jul 12, 2022

Merging since @youngnick already /approve.

@danehans danehans merged commit 4192e8d into envoyproxy:main Jul 12, 2022
@danehans danehans deleted the cfg_api_impl branch July 12, 2022 19:03
@danehans
Copy link
Copy Markdown
Contributor Author

danehans commented Jul 12, 2022

Fixes: #146

This was referenced Jul 12, 2022
Closed
Closed
Closed
@LukeShu LukeShu mentioned this pull request Jul 12, 2022
Merged
@danehans danehans mentioned this pull request Jul 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arkodg arkodg arkodg approved these changes

+2 more reviewers

@kflynn kflynn kflynn left review comments

@youngnick youngnick youngnick left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@danehans @codecov-commenter @kflynn @youngnick @arkodg