PMM-11423: support for updating a service #1590

michal-kralik · 2023-01-19T10:39:59Z

Requires the following to be merged at the same time:

codecov · 2023-01-19T10:44:00Z

Codecov Report

Merging #1590 (b43a5f6) into main (5d52bd6) will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1590      +/-   ##
==========================================
+ Coverage   42.81%   42.85%   +0.03%     
==========================================
  Files         382      382              
  Lines       48101    48085      -16     
==========================================
+ Hits        20595    20607      +12     
+ Misses      25568    25545      -23     
+ Partials     1938     1933       -5

Flag	Coverage Δ
admin	`10.41% <ø> (ø)`
agent	`52.88% <ø> (+0.16%)`	⬆️
vmproxy	`69.69% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
managed/cmd/pmm-managed/main.go	`0.00% <ø> (ø)`
managed/models/service_helpers.go	`63.23% <ø> (ø)`
managed/models/service_model.go	`80.43% <ø> (ø)`
managed/services/inventory/grpc/services_server.go	`0.00% <ø> (ø)`
managed/services/inventory/services.go	`63.15% <ø> (+0.44%)`	⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

managed/services/inventory/services.go

denisok · 2023-01-30T09:38:44Z

@PavelKhripkov @BupycHuk please review.

api/inventorypb/services.proto

PavelKhripkov · 2023-01-30T16:30:14Z

@michal-kralik we have some functionality related to cluster label. Probably we need some changes in other parts of the code before merging your changes. Let me check it.

upd: We really need changes otherwise our mongo backup logic will be broken.

api/inventorypb/services.proto

managed/models/service_model.go

managed/services/inventory/grpc/services_server.go

managed/services/inventory/services.go

managed/models/service_model.go

michal-kralik · 2023-01-31T13:13:04Z

@michal-kralik we have some functionality related to cluster label. Probably we need some changes in other parts of the code before merging your changes. Let me check it.

upd: We really need changes otherwise our mongo backup logic will be broken.

Sounds good. What kind of changes do you have in mind?

PavelKhripkov · 2023-02-02T10:27:26Z

@michal-kralik we have some functionality related to cluster label. Probably we need some changes in other parts of the code before merging your changes. Let me check it.
upd: We really need changes otherwise our mongo backup logic will be broken.

Sounds good. What kind of changes do you have in mind?

I'm waiting response from @rnovikovP regarding this thing. Need to set priority and discuss possible solutions

managed/models/service_model.go

managed/services/inventory/services.go

PavelKhripkov · 2023-02-03T08:56:17Z

@michal-kralik we have some functionality related to cluster label. Probably we need some changes in other parts of the code before merging your changes. Let me check it.
upd: We really need changes otherwise our mongo backup logic will be broken.

Sounds good. What kind of changes do you have in mind?

I'm waiting response from @rnovikovP regarding this thing. Need to set priority and discuss possible solutions

upd: We decided to not block these changes and we'll prepare fixes to cover cases when cluster is changed.

managed/models/service_helpers.go

Remove scheduled tasks on cluster changing.

michal-kralik · 2023-02-08T10:59:23Z

managed/services/inventory/services.go

@@ -434,7 +435,11 @@ func (ss *ServicesService) RemoveCustomLabels(ctx context.Context, req *inventor
 }

 // ChangeService changes service configuration.
-func (ss *ServicesService) ChangeService(ctx context.Context, params *models.ChangeStandardLabelsParams) (*inventorypb.ChangeServiceResponse, error) {
+func (ss *ServicesService) ChangeService(ctx context.Context, mgmtServices common.MgmtServices, params *models.ChangeStandardLabelsParams) error {
+	if err := mgmtServices.RemoveScheduledTasks(ctx, ss.db, params); err != nil {


@PavelKhripkov would it make more sense to execute this in a transaction along with the labels change?
In case the labels change fail, we already removed scheduled tasks and there's no way back.

For sure, but the calls made on api-level that doesn't allow to rollback. To implement the same on a deeper level, we need to duplicate alot of code lines. And without proper testing It won't be reliable.

…tandard-labels

managed/services/management/common/common.go

managed/services/inventory/grpc/services_server.go

PMM-11423: support for updating a service

367690c

michal-kralik requested review from a team and BupycHuk as code owners January 19, 2023 10:40

michal-kralik requested review from PavelKhripkov and removed request for a team January 19, 2023 10:40

michal-kralik mentioned this pull request Jan 19, 2023

PMM-11423: update service labels Percona-Lab/pmm-submodules#3016

Closed

BupycHuk reviewed Jan 23, 2023

View reviewed changes

managed/services/inventory/services.go Outdated Show resolved Hide resolved

michal-kralik added 2 commits January 23, 2023 11:35

Refactor update of labels to a model

7b5f0cb

Merge branch 'main' into PMM-11423-update-standard-labels

27b2d32

michal-kralik requested a review from BupycHuk January 23, 2023 10:35

JNKPercona mentioned this pull request Jan 24, 2023

PMM-11148-inventory-manage-labels (FB) Percona-Lab/pmm-submodules#3026

Closed

Merge branch 'main' into PMM-11423-update-standard-labels

3b9d97b

BupycHuk requested review from a team, gen1us2k and percona-csalguero and removed request for a team January 30, 2023 09:39

BupycHuk reviewed Jan 30, 2023

View reviewed changes

api/inventorypb/services.proto Outdated Show resolved Hide resolved

Rename UpdateService to ChangeService

7d1c3c0

michal-kralik requested a review from BupycHuk January 30, 2023 14:34

PavelKhripkov reviewed Jan 31, 2023

View reviewed changes

michal-kralik added 2 commits January 31, 2023 14:05

Change API comments

bb07ef5

Split business and model logic

f9d0806

michal-kralik requested a review from PavelKhripkov January 31, 2023 13:12

PavelKhripkov reviewed Feb 2, 2023

View reviewed changes

managed/models/service_model.go Outdated Show resolved Hide resolved

managed/services/inventory/services.go Outdated Show resolved Hide resolved

Merge branch 'main' into PMM-11423-update-standard-labels

43040dc

michal-kralik requested a review from PavelKhripkov February 6, 2023 10:39

percona-csalguero approved these changes Feb 6, 2023

View reviewed changes

Merge branch 'main' into PMM-11423-update-standard-labels

a9716c8

PavelKhripkov suggested changes Feb 6, 2023

View reviewed changes

managed/models/service_helpers.go Show resolved Hide resolved

managed/models/service_helpers.go Show resolved Hide resolved

michal-kralik added 2 commits February 7, 2023 11:35

Add tests for changing standard labels

4335370

Merge branch 'main' into PMM-11423-update-standard-labels

c4bcfbd

michal-kralik requested a review from PavelKhripkov February 7, 2023 10:36

PavelKhripkov approved these changes Feb 7, 2023

View reviewed changes

PavelKhripkov and others added 2 commits February 7, 2023 19:32

PMM-11546 remove scheduled backups when cluster changed (#1662)

8b52e00

Remove scheduled tasks on cluster changing.

Merge branch 'main' into PMM-11423-update-standard-labels

1419049

michal-kralik commented Feb 8, 2023

View reviewed changes

michal-kralik added 2 commits February 16, 2023 10:24

Merge branch 'main' into PMM-11423-update-standard-labels

0004743

Merge branch 'main' into PMM-11423-update-standard-labels

c7f5282

gen1us2k approved these changes Mar 23, 2023

View reviewed changes

Alex Tymchuk added 2 commits July 7, 2023 17:30

Merge branch 'main' of github.com:percona/pmm into PMM-11423-update-s…

d675dcb

…tandard-labels

PMM-11423 fix conflicts

bb89338

github-actions bot reviewed Jul 7, 2023

View reviewed changes

managed/services/management/common/common.go Outdated Show resolved Hide resolved

managed/services/inventory/grpc/services_server.go Outdated Show resolved Hide resolved

Alex Tymchuk and others added 7 commits July 7, 2023 18:30

PMM-11423 fix linter errors

410d76e

Merge branch 'main' into PMM-11423-update-standard-labels

cf0e707

Merge branch 'main' into PMM-11423-update-standard-labels

a4acb17

Merge branch 'main' into PMM-11423-update-standard-labels

85d660f

Merge branch 'main' into PMM-11423-update-standard-labels

d2c1844

Merge branch 'main' into PMM-11423-update-standard-labels

1d07cb9

Merge branch 'main' into PMM-11423-update-standard-labels

e28b056

BupycHuk approved these changes Aug 25, 2023

View reviewed changes

matejkubinec added 2 commits August 25, 2023 18:08

Merge branch 'main' into PMM-11423-update-standard-labels

969c028

Merge branch 'main' into PMM-11423-update-standard-labels

b43a5f6

matejkubinec merged commit 12a3523 into main Aug 28, 2023
32 checks passed

matejkubinec deleted the PMM-11423-update-standard-labels branch August 28, 2023 11:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PMM-11423: support for updating a service #1590

PMM-11423: support for updating a service #1590

michal-kralik commented Jan 19, 2023 •

edited

Loading

codecov bot commented Jan 19, 2023 •

edited

Loading

denisok commented Jan 30, 2023

PavelKhripkov commented Jan 30, 2023 •

edited

Loading

michal-kralik commented Jan 31, 2023

PavelKhripkov commented Feb 2, 2023

PavelKhripkov commented Feb 3, 2023

michal-kralik Feb 8, 2023

PavelKhripkov Feb 8, 2023

PMM-11423: support for updating a service #1590

PMM-11423: support for updating a service #1590

Conversation

michal-kralik commented Jan 19, 2023 • edited Loading

codecov bot commented Jan 19, 2023 • edited Loading

Codecov Report

denisok commented Jan 30, 2023

PavelKhripkov commented Jan 30, 2023 • edited Loading

michal-kralik commented Jan 31, 2023

PavelKhripkov commented Feb 2, 2023

PavelKhripkov commented Feb 3, 2023

michal-kralik Feb 8, 2023

Choose a reason for hiding this comment

PavelKhripkov Feb 8, 2023

Choose a reason for hiding this comment

michal-kralik commented Jan 19, 2023 •

edited

Loading

codecov bot commented Jan 19, 2023 •

edited

Loading

PavelKhripkov commented Jan 30, 2023 •

edited

Loading