MGMT-19338, MGMT-19080, MGMT-18590, NO-ISSUE: Patches for non-standard HA OCP Control Plane PR #7023
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@danmanor: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
size/XL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danmanor The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/test |
|
@danmanor: The The following commands are available to trigger optional jobs: Use DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test edge-e2e-ai-operator-ztp-4masters |
|
/test e2e-agent-4control-ipv4 |
|
/test edge-e2e-metal-assisted-4-control-planes-4-18 |
danmanor
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7023 +/- ##
==========================================
+ Coverage 68.21% 68.27% +0.06%
==========================================
Files 273 274 +1
Lines 39075 39127 +52
==========================================
+ Hits 26655 26715 +60
+ Misses 10006 9995 -11
- Partials 2414 2417 +3
|
|
I'm missing tests for SNO |
eifrach
left a comment
eifrach
left a comment
There was a problem hiding this comment.
please confirm that we have at least one SNO test green
...1_update_new_column_control_plane_count_value_for_existing_multinode_cluster_records_test.go
Outdated
Show resolved
Hide resolved
openshift-ci
bot
added
size/XXL
openshift-ci
bot
added
size/XL
|
/test edge-e2e-ai-operator-ztp-4masters /test e2e-agent-4control-ipv4 edge-e2e-metal-assisted-4-control-planes-4-18 edge-e2e-ai-operator-ztp-sno-day2-workers |
|
@eifrach Worked in integration |
|
/test edge-unit-test |
danmanor
force-pushed
the
migration
branch
2 times, most recently
from
b8539b4 to
a7e9631
Compare
|
/test edge-e2e-ai-operator-ztp-4masters /test e2e-agent-4control-ipv4 edge-e2e-metal-assisted-4-control-planes-4-18 edge-e2e-ai-operator-ztp-sno-day2-workers |
|
/retest |
|
/override ci/prow/okd-scos-e2e-aws-ovn ci/prow/edge-e2e-ai-operator-disconnected-capi |
|
@danmanor: Overrode contexts on behalf of danmanor: ci/prow/edge-e2e-ai-operator-disconnected-capi, ci/prow/okd-scos-e2e-aws-ovn DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cc @tsorya |
|
disconnected-capi is broken |
| // control_plane_count value in existing records can be NULL or 0 (default). We want to set the value in both cases | ||
| Where(cleanQuery.Where("control_plane_count IS NULL").Or("control_plane_count = ?", 0)). | ||
| Where("high_availability_mode = ?", models.ClusterCreateParamsHighAvailabilityModeFull). | ||
| Update("control_plane_count", "3").Error |
There was a problem hiding this comment.
What will happen if we have a cluster without hosts and the high availability is full? Do we still want to update it to 3, or should it be 0?
There was a problem hiding this comment.
We want it to be 3 as it represents the desired amount of hosts
There was a problem hiding this comment.
Why we need need to migrate installed clusters? I mean this field is not relevant for them, no?
How it will influence ACM or we don't run migration in ACM flow?
internal/common/db.go
Outdated
|
|
||
| func GetHostCountByRole(db *gorm.DB, clusterID strfmt.UUID, role models.HostRole, suggested bool) (*int64, error) { | ||
| // Start from empty query | ||
| cleanQuery := db.Session(&gorm.Session{NewDB: true}) |
There was a problem hiding this comment.
why session or why NewDB: true ?
| return models.SupportLevelUnavailable | ||
| } | ||
|
|
||
| if filters.PlatformType != nil && *filters.PlatformType != models.PlatformTypeBaremetal { |
There was a problem hiding this comment.
not supported in None? Why only Baremetal
There was a problem hiding this comment.
There was a problem hiding this comment.
Probably can work with none as well
There was a problem hiding this comment.
Just not nutanix, vspere, etc
...s/20241122160000_update_new_column_control_plane_count_value_for_existing_cluster_records.go
Show resolved
Hide resolved
...41122160000_update_new_column_control_plane_count_value_for_existing_cluster_records_test.go
Show resolved
Hide resolved
|
/test edge-e2e-ai-operator-ztp-4masters /test e2e-agent-4control-ipv4 edge-e2e-metal-assisted-4-control-planes-4-18 edge-e2e-ai-operator-ztp-sno-day2-workers |
danmanor
force-pushed
the
migration
branch
2 times, most recently
from
295a533 to
c3c74f8
Compare
|
/test edge-e2e-ai-operator-ztp-4masters /test e2e-agent-4control-ipv4 edge-e2e-metal-assisted-4-control-planes-4-18 edge-e2e-ai-operator-ztp-sno-day2-workers |
|
/lgtm |
|
@danmanor: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
[ART PR BUILD NOTIFIER] Distgit: ose-agent-installer-api-server |
6 participants
This PR continues #6917:
List all the issues related to this PR
What environments does this code impact?
How was this code tested?
Checklist
docs, README, etc)Reviewers Checklist