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.

Sign up for GitHub

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

Jump to bottom

test: stabilize case single_prometheus_replica_has_no_pdb #425

Merged
merged 2 commits into from
Feb 28, 2024
Merged

test: stabilize case single_prometheus_replica_has_no_pdb #425

merged 2 commits into from
Feb 28, 2024

Conversation

lihongyan1
Copy link
Contributor

Stabilize case: TestMonitoringStackController/single_prometheus_replica_has_no_pdb
fix error "the object has been modified; please apply your changes to the latest version and try again: failed to update monitoring stack" tracked by https://issues.redhat.com/browse/COO-42

@lihongyan1 lihongyan1 requested a review from a team as a code owner February 21, 2024 09:29
@lihongyan1 lihongyan1 requested review from sthaha and slashpai and removed request for a team February 21, 2024 09:29
@lihongyan1 lihongyan1 force-pushed the stablize-singlePrometheusReplicaHasNoPDB branch 2 times, most recently from b7a28f2 to 43653da Compare February 21, 2024 09:43
@jan--f
Copy link
Collaborator

jan--f commented Feb 21, 2024

Thanks! This is great. Would you be up for moving this to a func (f *Framework) UpdateWithRetry err function and replace all the update calls in our tests with that? I think I have seen this warning/error in other test runs before too.

I'd also be happy to merge this as is and refactor later. Up to you.

@lihongyan1
Copy link
Contributor Author

Thanks! This is great. Would you be up for moving this to a func (f *Framework) UpdateWithRetry err function and replace all the update calls in our tests with that? I think I have seen this warning/error in other test runs before too.

I'd also be happy to merge this as is and refactor later. Up to you.

Will refactor the code

@jan--f jan--f changed the title test: stalize case single_prometheus_replica_has_no_pdb test: stabilize case single_prometheus_replica_has_no_pdb Feb 22, 2024
@lihongyan1 lihongyan1 force-pushed the stablize-singlePrometheusReplicaHasNoPDB branch 3 times, most recently from 656ded2 to 6a6bcd1 Compare February 26, 2024 02:44
@lihongyan1
Copy link
Contributor Author

@jan--f complete refactor code, please help review.

jan--f
jan--f reviewed Feb 27, 2024
View reviewed changes
Copy link
Collaborator

@jan--f jan--f left a comment

Choose a reason for hiding this comment

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

Two small comments, otherwise this looks great. Thanks!

test/e2e/framework/monitoring_stack.go Outdated

type StackFn func(monitoringStack *stack.MonitoringStack)

func SetPrometheusReplicas(replicas *int32) StackFn {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe

Suggested change
func SetPrometheusReplicas(replicas *int32) StackFn {
func SetPrometheusReplicas(replicas int32) StackFn {

And get the pointer to the int in the set function? Calls are more convenient that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can't agree more

test/e2e/framework/monitoring_stack.go Outdated
"k8s.io/client-go/util/retry"
)

type StackFn func(monitoringStack *stack.MonitoringStack)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: maybe StackModifier is a slightly more descriptive name?

@openshift-ci openshift-ci bot removed the approved label Feb 28, 2024
@lihongyan1
test: stalize monitoringstack cases
7aa4e6c 
  Refactor code and also enhance the following cases
  TestMonitoringStackController/single_prometheus_replica_has_no_pdb
  TestMonitoringStackController/Alertmanager_disabled
  TestMonitoringStackController/Verify_ability_to_scale_down_Prometheus
@lihongyan1 lihongyan1 force-pushed the stablize-singlePrometheusReplicaHasNoPDB branch from 7fd2973 to 9ed6838 Compare February 28, 2024 03:38
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Feb 28, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lihongyan1

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lihongyan1 lihongyan1 force-pushed the stablize-singlePrometheusReplicaHasNoPDB branch from 9ed6838 to 0efe1f1 Compare February 28, 2024 07:11
@lihongyan1 lihongyan1 force-pushed the stablize-singlePrometheusReplicaHasNoPDB branch from 0efe1f1 to 4bd23b7 Compare February 28, 2024 07:13
@jan--f
Copy link
Collaborator

jan--f commented Feb 28, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Feb 28, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 463f767 into rhobs:main Feb 28, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jan--f jan--f jan--f left review comments

@sthaha sthaha Awaiting requested review from sthaha sthaha is a code owner automatically assigned from rhobs/observability-operator-maintainers

@slashpai slashpai Awaiting requested review from slashpai slashpai is a code owner automatically assigned from rhobs/observability-operator-maintainers

@simonpasquier simonpasquier Awaiting requested review from simonpasquier

@tremes tremes Awaiting requested review from tremes

Assignees

@jan--f jan--f

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lihongyan1 @jan--f