Skip to content

observability: add container port for metrics#1736

Merged
arkodg merged 1 commit intoenvoyproxy:mainfrom
zirain:monitoring-pod
Aug 1, 2023
Merged

observability: add container port for metrics#1736
arkodg merged 1 commit intoenvoyproxy:mainfrom
zirain:monitoring-pod

Conversation

@zirain
Copy link
Copy Markdown
Member

@zirain zirain commented Aug 1, 2023

Signed-off-by: zirain <[email protected]>
@zirain zirain requested a review from a team as a code owner August 1, 2023 08:04
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 1, 2023

Codecov Report

Merging #1736 (1c81b78) into main (9ba9103) will increase coverage by 0.02%.
Report is 5 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1736      +/-   ##
==========================================
+ Coverage   64.86%   64.89%   +0.02%     
==========================================
  Files          83       83              
  Lines       11924    11932       +8     
==========================================
+ Hits         7735     7743       +8     
  Misses       3707     3707              
  Partials      482      482              
Files Changed Coverage Δ
...ternal/infrastructure/kubernetes/proxy/resource.go 93.50% <100.00%> (+0.27%) ⬆️

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 !

@arkodg arkodg added this to the 0.5.0 milestone Aug 1, 2023
@arkodg arkodg requested review from a team, LanceEa, Xunzhuo and qicz and removed request for a team August 1, 2023 16:55
Copy link
Copy Markdown
Contributor

@LanceEa LanceEa 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!

@arkodg arkodg merged commit 4bba03a into envoyproxy:main Aug 1, 2023
@zirain zirain deleted the monitoring-pod branch August 1, 2023 22:43
arkodg pushed a commit to arkodg/gateway that referenced this pull request Aug 2, 2023
container port

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 4bba03a)
arkodg added a commit that referenced this pull request Aug 2, 2023
* refactor: set defaults in Deployment, else k8s sets them for you, creating infinite reconciliation loop (#1594)

* fix: envoy proxy resource apply bug.

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

* update pointer.

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

* add comment

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

* update cm cmp logic.

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

* fix lint

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

* add probe field default value.

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

* fix uts

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

* align probe

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

* optimize deploy compare logic

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

* add compare deploy uts

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

* rm cm binarydata cmp

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

* rm deploy cmp logic

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

* fix ut

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

* fix lint

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

---------

Signed-off-by: qicz <[email protected]>
Signed-off-by: qi <[email protected]>
(cherry picked from commit 9ba9103)

* DeepCopy resources that require status updates (#1723)

* Was seeing constant churn between provider runner publishing resources
and gateway-api runner receiving them.

* Tried to debug it by printing the o/p of `cmp.Diff` between current
  and previous values
```
diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go
index 050394b..50d09f6f 100644
--- a/internal/gatewayapi/runner/runner.go
+++ b/internal/gatewayapi/runner/runner.go
@@ -8,6 +8,7 @@ package runner
 import (
        "context"

+       "github.com/google/go-cmp/cmp"
        "k8s.io/apimachinery/pkg/runtime/schema"
        "sigs.k8s.io/gateway-api/apis/v1beta1"
        "sigs.k8s.io/yaml"
@@ -49,6 +50,7 @@ func (r *Runner) Start(ctx context.Context) error {
 }

 func (r *Runner) subscribeAndTranslate(ctx context.Context) {
+       prev := &gatewayapi.Resources{}
        message.HandleSubscription(r.ProviderResources.GatewayAPIResources.Subscribe(ctx),
                func(update message.Update[string, *gatewayapi.Resources]) {
                        val := update.Value
@@ -56,6 +58,9 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) {
                        if update.Delete || val == nil {
                                return
                        }
+                       diff := cmp.Diff(prev, val)
+                       r.Logger.WithValues("output", "diff").Info(diff)
+                       prev = val.DeepCopy()

                        // Translate and publish IRs.
                        t := &gatewayapi.Translator{
```

Here's the o/p and its empty
```
2023-07-27T23:55:29.795Z	INFO	gateway-api	runner/runner.go:62		{"runner": "gateway-api", "output": "diff"}
```

* Using a DeepCopy for resources that were updating the `Status`
  subresource seems to have solved the issue, which implies that
  watchable doesnt like clients to mutate the value, even though they
  are meant to be a `DeepCopy`

Fixes: #1715

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit 5b72451)

* observability: add container port for metrics (#1736)

container port

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 4bba03a)

* docs: Add user docs for EnvoyPatchPolicy (#1733)

* Add user docs for EnvoyPatchPolicy

Relates to #24

Signed-off-by: Arko Dasgupta <[email protected]>

* nits

Signed-off-by: Arko Dasgupta <[email protected]>

* wrap up

Signed-off-by: Arko Dasgupta <[email protected]>

* lint

Signed-off-by: Arko Dasgupta <[email protected]>

* address comments && fix config

Signed-off-by: Arko Dasgupta <[email protected]>

---------

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit 27b0939)

* e2e & misc fixes for EnvoyPatchPolicy (#1738)

* Add E2E for EnvoyPatchPolicy

* Use LocalReplyConfig to return a custom
status code `406` when there is no valid route match

Signed-off-by: Arko Dasgupta <[email protected]>
(cherry picked from commit a7784c5)

---------

Signed-off-by: Arko Dasgupta <[email protected]>
Co-authored-by: qi <[email protected]>
Co-authored-by: zirain <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants