Add metrics proxy #2182

vthapar · 2022-08-18T07:09:03Z

Refer submariner-io/enhancements/pull/128
Depends on submariner-io/subctl/pull/212
Depends on #2209

Signed-off-by: Vishal Thapar [email protected]

submariner-bot · 2022-08-18T07:09:07Z

🤖 Created branch: z_pr2182/vthapar/metrics-proxy
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

With the redesign of metrics, we don't need to open metrics port in the cloud anymore. This also marks the `metrics-ports` flag as deprecated. Refer submariner-io/enhancements/pull/128 Depends On submariner-io/submariner-operator/pull/2182 Signed-off-by: Vishal Thapar <[email protected]>

controllers/servicediscovery/servicediscovery_controller.go

controllers/submariner/metrics_proxy_resources.go

controllers/submariner/submariner_controller.go

controllers/submariner/gateway_resources.go

Changes addressed

mkolesnik

A few suggestions, overall looks good

controllers/submariner/metrics_proxy_resources.go

controllers/constants/constants.go

mkolesnik · 2022-08-23T13:28:58Z

controllers/submariner/submariner_controller.go

@@ -227,6 +231,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 		return reconcile.Result{}, err
 	}

+	// TODO: vthapar Do we need proxy status?


Seems that for consistency we should, no?

Status bits are currently added to SUbmariner CR's status field. This will require additions to Submariner CR in submariner repo. We're also missing status for NetworkPluginSyncer. Should we bring them both in a separate issue?

I dont think syncer should be part of it, it's a separate issue

Yup, we shouldn’t take care of the syncer here, but I agree we should add the metrics status information here while we’re at it.

@skitt @mkolesnik Code here is to update Submariner CR's Status fields with DaemonSet's status. Till we add the CR fields in submariner we can't do anything here.

controllers/submariner/globalnet_resources.go

controllers/submariner/gateway_resources.go

With the redesign of metrics, we don't need to open metrics port in the cloud anymore. This also marks the `metrics-ports` flag as deprecated. Refer submariner-io/enhancements/pull/128 Depends On submariner-io/submariner-operator/pull/2182 Signed-off-by: Vishal Thapar <[email protected]>

controllers/constants/constants.go

mkolesnik · 2022-08-25T08:38:14Z

controllers/submariner/submariner_controller.go

@@ -227,6 +231,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 		return reconcile.Result{}, err
 	}

+	// TODO: vthapar Do we need proxy status?


I dont think syncer should be part of it, it's a separate issue

controllers/submariner/globalnet_resources.go

controllers/submariner/submariner_controller.go

controllers/submariner/metrics_proxy_resources.go

controllers/submariner/gateway_resources.go

vthapar · 2022-09-06T07:19:57Z

I manually squashed older review commits into parent commit to make it easier to rebase conflicts.

mkolesnik

LGTM

skitt · 2022-09-06T07:53:12Z

@vthapar please squash the latest review commit.

mkolesnik

Please add release notes as part of the PR :)

vthapar · 2022-09-07T04:25:21Z

Please add release notes as part of the PR

@mkolesnik Release notes should be added as new comment or updated in the parent comment?

mkolesnik · 2022-09-07T09:46:00Z

Please add release notes as part of the PR

@mkolesnik Release notes should be added as new comment or updated in the parent comment?

It should be part of the PR - as we discussed on the weekly calls, and the pending documentation (submariner-io/submariner-website#802)

vthapar · 2022-09-07T14:17:38Z

Please add release notes as part of the PR

@mkolesnik Release notes should be added as new comment or updated in the parent comment?

It should be part of the PR - as we discussed on the weekly calls, and the pending documentation (submariner-io/submariner-website#802)

I saw the documentation PR but that is still under review. I don't see a release-notes directory in operator repo. Also, what should be the date in filename YYYYMMDD format? Date release note is added to PR?

tpantelis · 2022-09-08T11:44:50Z

@vthapar The release notes file has linting errors.

Refer submariner-io/enhancements/pull/128 Depends On submariner-io/subctl/pull/212 Signed-off-by: Vishal Thapar <[email protected]>

submariner-bot · 2022-09-08T13:02:31Z

🤖 Closed branches: [z_pr2182/vthapar/metrics-proxy]

vthapar mentioned this pull request Aug 18, 2022

Investigate exporting Submariner metrics without needing to open ports submariner-io/submariner#1944

Closed

5 tasks

vthapar force-pushed the metrics-proxy branch from 3e51e77 to 016221d Compare August 18, 2022 07:18

vthapar requested a review from mkolesnik August 18, 2022 07:19

vthapar marked this pull request as ready for review August 18, 2022 09:17

vthapar requested review from Oats87, skitt, sridhargaddam and tpantelis as code owners August 18, 2022 09:17

vthapar mentioned this pull request Aug 19, 2022

cloud-prepare: Don't open metrics port submariner-io/subctl#228

Merged

mkolesnik previously requested changes Aug 21, 2022

View reviewed changes

vthapar force-pushed the metrics-proxy branch from 016221d to 68c813c Compare August 22, 2022 10:38

vthapar requested a review from mkolesnik August 22, 2022 11:28

vthapar force-pushed the metrics-proxy branch from 68c813c to 7b79a1c Compare August 22, 2022 11:31

vthapar force-pushed the metrics-proxy branch from 7b79a1c to 9c3fcce Compare August 23, 2022 04:47

mkolesnik reviewed Aug 23, 2022

View reviewed changes

vthapar force-pushed the metrics-proxy branch from b964167 to e8b93c1 Compare August 24, 2022 07:12

vthapar requested a review from mkolesnik August 24, 2022 07:15

vthapar force-pushed the metrics-proxy branch from e8b93c1 to 880a9bf Compare August 25, 2022 06:13

mkolesnik reviewed Aug 25, 2022

View reviewed changes

vthapar force-pushed the metrics-proxy branch from 880a9bf to cd41f43 Compare August 25, 2022 12:55

vthapar requested a review from mkolesnik August 25, 2022 14:35

vthapar force-pushed the metrics-proxy branch from cd41f43 to 943ea5a Compare August 26, 2022 08:22

vthapar force-pushed the metrics-proxy branch from d39f841 to 778209a Compare September 2, 2022 06:40

mkolesnik reviewed Sep 4, 2022

View reviewed changes

controllers/submariner/metrics_proxy_resources.go Outdated Show resolved Hide resolved

controllers/submariner/gateway_resources.go Outdated Show resolved Hide resolved

vthapar force-pushed the metrics-proxy branch from 778209a to 266502b Compare September 6, 2022 04:15

vthapar requested a review from mkolesnik September 6, 2022 05:33

mkolesnik approved these changes Sep 6, 2022

View reviewed changes

skitt approved these changes Sep 6, 2022

View reviewed changes

submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Sep 6, 2022

mkolesnik requested changes Sep 6, 2022

View reviewed changes

vthapar force-pushed the metrics-proxy branch from 266502b to 982bfdc Compare September 7, 2022 04:21

tpantelis approved these changes Sep 7, 2022

View reviewed changes

tpantelis enabled auto-merge (rebase) September 7, 2022 17:12

vthapar requested a review from dfarrell07 as a code owner September 8, 2022 10:41

mkolesnik approved these changes Sep 8, 2022

View reviewed changes

vthapar force-pushed the metrics-proxy branch 2 times, most recently from 1f56a75 to ed938a7 Compare September 8, 2022 12:13

Add metrics proxy

3667413

Refer submariner-io/enhancements/pull/128 Depends On submariner-io/subctl/pull/212 Signed-off-by: Vishal Thapar <[email protected]>

vthapar force-pushed the metrics-proxy branch from ed938a7 to 3667413 Compare September 8, 2022 12:19

tpantelis merged commit 0104812 into submariner-io:devel Sep 8, 2022

nyechiel mentioned this pull request Sep 29, 2022

Document the new metrics proxy changes submariner-io/submariner-website#829

Closed

3 tasks

maayanf24 added the release-note-handled label Oct 25, 2022

dfarrell07 removed release-note-needed Should be mentioned in the release notes release-note-handled labels May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics proxy #2182

Add metrics proxy #2182

vthapar commented Aug 18, 2022 •

edited by skitt

Loading

submariner-bot commented Aug 18, 2022

mkolesnik left a comment

mkolesnik Aug 23, 2022

vthapar Aug 24, 2022

mkolesnik Aug 25, 2022

skitt Aug 25, 2022

vthapar Sep 6, 2022

mkolesnik Aug 25, 2022

vthapar commented Sep 6, 2022

mkolesnik left a comment

skitt commented Sep 6, 2022

mkolesnik left a comment

vthapar commented Sep 7, 2022

mkolesnik commented Sep 7, 2022

vthapar commented Sep 7, 2022 •

edited

Loading

tpantelis commented Sep 8, 2022

submariner-bot commented Sep 8, 2022

Add metrics proxy #2182

Add metrics proxy #2182

Conversation

vthapar commented Aug 18, 2022 • edited by skitt Loading

submariner-bot commented Aug 18, 2022

mkolesnik left a comment

Choose a reason for hiding this comment

mkolesnik Aug 23, 2022

Choose a reason for hiding this comment

vthapar Aug 24, 2022

Choose a reason for hiding this comment

mkolesnik Aug 25, 2022

Choose a reason for hiding this comment

skitt Aug 25, 2022

Choose a reason for hiding this comment

vthapar Sep 6, 2022

Choose a reason for hiding this comment

mkolesnik Aug 25, 2022

Choose a reason for hiding this comment

vthapar commented Sep 6, 2022

mkolesnik left a comment

Choose a reason for hiding this comment

skitt commented Sep 6, 2022

mkolesnik left a comment

Choose a reason for hiding this comment

vthapar commented Sep 7, 2022

mkolesnik commented Sep 7, 2022

vthapar commented Sep 7, 2022 • edited Loading

tpantelis commented Sep 8, 2022

submariner-bot commented Sep 8, 2022

vthapar commented Aug 18, 2022 •

edited by skitt

Loading

vthapar commented Sep 7, 2022 •

edited

Loading