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

observability: add container port for metrics #1736

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

zirain
Copy link
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

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
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 LanceEa, a team, qicz and Xunzhuo and removed request for a team August 1, 2023 16:55
Copy link
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 050394ba..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