Adds Initial Infra Manager by danehans · Pull Request #178 · envoyproxy/gateway

danehans · 2022-07-28T16:04:49Z

Adds initial Infra Manager implementation.

Requires: #183

xref: #41

Signed-off-by: danehans [email protected]

arkodg · 2022-07-28T16:35:08Z

internal/infrastructure/translator.go

imho the metadata should only be used for informational features such as labelling and status and not for CRUD of k8s resources.
one way to handle deletes could be for the infra manager to reconcile all k8s resources owned by this infra manager (tied to a single GW Class), and delete those resources that are not part of the Translate o/p

IMO infra IR metadata should include a deletionTimestamp that is set by the Resource Translator when the associated Gateway is deleted. An infra IR with deletionTimestamp tells the Infra Manager to delete the associated infra.

xref kube meta: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata

one way to handle deletes could be for the infra manager to reconcile all k8s resources owned by this infra manager (tied to a single GW Class), and delete those resources that are not part of the Translate o/p

Yes, this is a possible solution but IMO not as efficient as it requires a List() and then Delete().

internal/infrastructure/kubernetes/infra.go

internal/infrastructure/kubernetes/infra_test.go

LukeShu · 2022-07-28T21:39:25Z

internal/infrastructure/manager.go

Do we expect there to multiple things here? Before I got to this file, I was assuming Manager was going to be an interface that kubernetes.Infra implements.

Yes, the struct will contain a field for every supported provider. I created #182 to track the need to move this to an interface.

internal/infrastructure/translator.go

codecov-commenter · 2022-07-29T21:58:37Z

Codecov Report

Merging #178 (cea006f) into main (a1ff4f9) will decrease coverage by 1.58%.
The diff coverage is 44.13%.

@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
- Coverage   61.74%   60.16%   -1.59%     
==========================================
  Files          20       23       +3     
  Lines        1814     1993     +179     
==========================================
+ Hits         1120     1199      +79     
- Misses        632      722      +90     
- Partials       62       72      +10

Impacted Files	Coverage Δ
internal/cmd/server.go	`24.32% <ø> (ø)`
...ternal/infrastructure/kubernetes/serviceaccount.go	`21.42% <21.42%> (ø)`
internal/infrastructure/kubernetes/infra.go	`35.13% <35.13%> (ø)`
internal/ir/infra.go	`62.79% <62.79%> (ø)`

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

internal/infrastructure/kubernetes/infra.go

LukeShu · 2022-08-03T05:44:43Z

internal/infrastructure/kubernetes/infra.go

nit: this line is unnecessary

LukeShu

lgtm

internal/infrastructure/translator.go

internal/infrastructure/kubernetes/infra.go

internal/infrastructure/manager.go

internal/infrastructure/kubernetes/serviceaccount.go

Signed-off-by: danehans <[email protected]>

skriss

👍

danehans requested a review from a team as a code owner July 28, 2022 16:04

danehans added the area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. label Jul 28, 2022

danehans added this to the 0.2.0-rc1 milestone Jul 28, 2022

arkodg reviewed Jul 28, 2022

View reviewed changes

danehans force-pushed the infra_mgr branch from a5712ee to c8d30fe Compare July 28, 2022 17:49

LukeShu reviewed Jul 28, 2022

View reviewed changes

internal/infrastructure/kubernetes/infra.go Outdated Show resolved Hide resolved

LukeShu reviewed Jul 28, 2022

View reviewed changes

danehans force-pushed the infra_mgr branch from c8d30fe to 00578a3 Compare July 29, 2022 21:49

danehans mentioned this pull request Jul 29, 2022

Convert Manager Struct to Interface #182

Closed

danehans force-pushed the infra_mgr branch from 00578a3 to cea006f Compare July 29, 2022 22:36

danehans requested review from LukeShu and arkodg August 1, 2022 16:17

arkodg reviewed Aug 1, 2022

View reviewed changes

internal/infrastructure/kubernetes/infra.go Outdated Show resolved Hide resolved

danehans force-pushed the infra_mgr branch from cea006f to 1113345 Compare August 1, 2022 17:56

This was referenced Aug 1, 2022

Add Envoy Deployment Support #185

Closed

Add Envoy Service Support #186

Closed

danehans force-pushed the infra_mgr branch from 1113345 to 51d4344 Compare August 1, 2022 18:25

arkodg previously approved these changes Aug 1, 2022

View reviewed changes

danehans mentioned this pull request Aug 2, 2022

Adds Initial Infra IR Support to Resource Translator #188

Merged

danehans dismissed arkodg’s stale review via af1c29c August 2, 2022 21:25

danehans force-pushed the infra_mgr branch from 51d4344 to af1c29c Compare August 2, 2022 21:25

LukeShu reviewed Aug 3, 2022

View reviewed changes

internal/infrastructure/kubernetes/infra.go Outdated

Copy link
Copy Markdown

Contributor

LukeShu Aug 3, 2022 •

edited

Loading

Choose a reason for hiding this comment

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

nit: this line is unnecessary

LukeShu previously approved these changes Aug 3, 2022

View reviewed changes

skriss suggested changes Aug 3, 2022

View reviewed changes

danehans added 2 commits August 3, 2022 09:35

Adds Initial Infra Manager

f77a34e

Signed-off-by: danehans <[email protected]>

Resolves Skriss Feedback

9cb9cd4

Signed-off-by: danehans <[email protected]>

danehans dismissed LukeShu’s stale review via 9cb9cd4 August 3, 2022 16:44

danehans force-pushed the infra_mgr branch from af1c29c to 9cb9cd4 Compare August 3, 2022 16:44

skriss approved these changes Aug 3, 2022

View reviewed changes

danehans merged commit a603060 into envoyproxy:main Aug 3, 2022

danehans deleted the infra_mgr branch August 3, 2022 20:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds Initial Infra Manager#178

Adds Initial Infra Manager#178
danehans merged 2 commits intoenvoyproxy:mainfrom
danehans:infra_mgr

danehans commented Jul 28, 2022 •

edited

Loading

Uh oh!

arkodg Jul 28, 2022 •

edited

Loading

Uh oh!

danehans Jul 28, 2022

Uh oh!

Uh oh!

Uh oh!

LukeShu Jul 28, 2022

Uh oh!

danehans Jul 29, 2022

Uh oh!

Uh oh!

codecov-commenter commented Jul 29, 2022 •

edited

Loading

Uh oh!

Uh oh!

LukeShu Aug 3, 2022 •

edited

Loading

Uh oh!

LukeShu left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skriss left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

danehans commented Jul 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arkodg Jul 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danehans Jul 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LukeShu Jul 28, 2022

Choose a reason for hiding this comment

Uh oh!

danehans Jul 29, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-commenter commented Jul 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

LukeShu Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LukeShu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skriss left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danehans commented Jul 28, 2022 •

edited

Loading

arkodg Jul 28, 2022 •

edited

Loading

codecov-commenter commented Jul 29, 2022 •

edited

Loading

LukeShu Aug 3, 2022 •

edited

Loading