Skip to content

Conversation

@abutcher
Copy link
Member

@abutcher abutcher commented Sep 3, 2021

@openshift-ci openshift-ci bot requested a review from abhinavdahiya September 3, 2021 15:28
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 3, 2021
pool: testOvirtPool(),
expectedMachineSetReplicas: map[string]int64{
fmt.Sprintf("%s-worker-0", testInfraID): 3,
fmt.Sprintf("%s-worker", testInfraID): 3,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is worrisome.

Copy link
Contributor

@abhinavdahiya abhinavdahiya Sep 3, 2021

Choose a reason for hiding this comment

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

just so i understand , previously they had -0 suffix even when there was only on machineset for a machinepool, but they have replaced the name to no suffix. and our worry is that it will affect our existing clusters as they will now have a new machineset and abandon the last one?

Copy link
Member Author

@abutcher abutcher Sep 3, 2021

Choose a reason for hiding this comment

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

Yeah, my worry is that if the naming changes we will replace machinesets for existing ovirt clusters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a function to preserve the suffix.

@codecov
Copy link

codecov bot commented Sep 3, 2021

Codecov Report

Merging #1526 (facc958) into master (aaaeeff) will decrease coverage by 0.00%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1526      +/-   ##
==========================================
- Coverage   41.59%   41.59%   -0.01%     
==========================================
  Files         336      336              
  Lines       30601    30619      +18     
==========================================
+ Hits        12729    12735       +6     
- Misses      16785    16797      +12     
  Partials     1087     1087              
Impacted Files Coverage Δ
pkg/remoteclient/fake.go 0.00% <0.00%> (ø)
pkg/controller/remotemachineset/ovirt.go 47.29% <100.00%> (+4.65%) ⬆️
pkg/operator/hive/hive.go 0.00% <0.00%> (ø)
pkg/operator/hive/clustersync.go 0.00% <0.00%> (ø)
pkg/operator/hive/hiveadmission.go 0.00% <0.00%> (ø)
pkg/operator/hive/hive_controller.go 0.00% <0.00%> (ø)

@abutcher
Copy link
Member Author

abutcher commented Sep 7, 2021

/retest

@2uasimojo
Copy link
Member

/retest

Most recent failures look like busted resume of CI pool clusters.

@abutcher
Copy link
Member Author

abutcher commented Sep 7, 2021

/retest

@abutcher
Copy link
Member Author

abutcher commented Sep 8, 2021

Trying out a rebase

@abutcher
Copy link
Member Author

abutcher commented Sep 8, 2021

/test e2e

1 similar comment
@abutcher
Copy link
Member Author

abutcher commented Sep 8, 2021

/test e2e

@abhinavdahiya
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 9, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, abutcher

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,abutcher]

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

@abutcher
Copy link
Member Author

abutcher commented Sep 9, 2021

/test e2e-pool

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@2uasimojo
Copy link
Member

/test e2e-pool

@2uasimojo
Copy link
Member

(#1534 should address the most recent flake)

@2uasimojo
Copy link
Member

2uasimojo commented Sep 9, 2021

/test e2e-pool

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

6 similar comments
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@abutcher
Copy link
Member Author

/test e2e-pool

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit e6a2ba3 into openshift:master Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants