OCPCLOUD-2995: Machine status conversion and syncing#365
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@damdo: This pull request references OCPCLOUD-2995 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. 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. |
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
WalkthroughAdds bidirectional Machine and MachineSet status conversion between CAPI and MAPI, renames MAPI condition helpers and updates their behavior, extends multi-version status/condition diffing, refactors Machine reconciliation into staged ensure/create/update/status flows, updates MachineSet condition conversion, and adds tests and fuzzers for status/address/phase/condition handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Reconciler
participant MS as MachineSync Controller
participant Conv as Converters
participant Diff as Diff Utils
participant API as Kubernetes API
rect rgb(235,245,255)
Reconciler->>MS: Reconcile(request)
MS->>API: Get source objects (CAPI/MAPI)
MS->>Conv: Convert source ↔ target (spec + status)
MS->>Diff: Diff(spec/metadata/providerSpec)
alt spec/meta changed
MS->>API: Create or Update target spec/metadata
end
MS->>Diff: hasStatusChanges?
alt status changed
MS->>Conv: setChanged*StatusFields (merge changed fields & use util.SetMAPICondition for MAPI)
MS->>API: Patch target status
else
MS-->>MS: Skip status patch
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
damdo
changed the title
damdo
force-pushed
the
machine-status-conversion-and-syncing
branch
3 times, most recently
from
20be64f to
ef363b0
Compare
|
@damdo: This pull request references OCPCLOUD-2995 which is a valid 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
removed
the
do-not-merge/work-in-progress
|
@damdo: This pull request references OCPCLOUD-2995 which is a valid 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/controllers/machinesync/machine_sync_controller.go (1)
1349-1354: Fix copy-paste error in provider spec parsing.Line 1354 incorrectly parses
a.Spec.ProviderSpec.Valuetwice instead of parsingb.Spec.ProviderSpec.Valuefor the second machine.Apply this fix to parse the correct provider spec:
- ps2, err := mapi2capi.AWSProviderSpecFromRawExtension(a.Spec.ProviderSpec.Value) + ps2, err := mapi2capi.AWSProviderSpecFromRawExtension(b.Spec.ProviderSpec.Value) if err != nil { return nil, fmt.Errorf("unable to parse second Machine API machine set providerSpec: %w", err) }
🧹 Nitpick comments (9)
pkg/conversion/mapi2capi/machine.go (1)
220-236: Tune Ready condition severity: Info when not failed; Error only on failure.Marking Ready with SeverityError for all non-running phases is too aggressive. Suggest Info except when Phase == Failed.
Apply this diff:
- Severity: func() clusterv1.ConditionSeverity { - if mapiMachine.Status.Phase != nil && *mapiMachine.Status.Phase == mapiv1beta1.PhaseRunning { - return clusterv1.ConditionSeverityNone - } - - return clusterv1.ConditionSeverityError - }(), + Severity: func() clusterv1.ConditionSeverity { + switch { + case mapiMachine.Status.Phase != nil && *mapiMachine.Status.Phase == mapiv1beta1.PhaseRunning: + return clusterv1.ConditionSeverityNone + case mapiMachine.Status.Phase != nil && *mapiMachine.Status.Phase == mapiv1beta1.PhaseFailed: + return clusterv1.ConditionSeverityError + default: + return clusterv1.ConditionSeverityInfo + } + }(),pkg/conversion/capi2mapi/machine.go (1)
178-196: Prefer using MAPI phase constant for “Deleting” mapping.Avoid hard-coded string to reduce typos.
Apply this diff:
- case "Deleted": - return ptr.To("Deleting") // Deleted is not supported in MAPI but we can stay in Deleting until the machine is fully removed. + case "Deleted": + return ptr.To(string(mapiv1.PhaseDeleting)) // Deleted is not supported in MAPI; approximate with Deleting.pkg/conversion/capi2mapi/machine_test.go (1)
118-132: Add coverage for HostName/Hostname mapping and DNS-type filtering.Strengthen the status conversion test to:
- Include a HostName address and assert NodeHostName in MAPI.
- Include an ExternalDNS and assert it is filtered out.
Apply this diff:
- capiMachine := capibuilder.Machine(). + capiMachine := capibuilder.Machine(). WithName("test-machine"). WithNamespace("test-namespace"). WithNodeRef(nodeRef). WithLastUpdated(lastUpdated). WithAddresses(clusterv1.MachineAddresses{ - {Type: clusterv1.MachineAddressType(corev1.NodeInternalIP), Address: "10.0.0.1"}, - {Type: clusterv1.MachineAddressType(corev1.NodeExternalIP), Address: "203.0.113.1"}, + {Type: clusterv1.MachineHostName, Address: "ip-10-0-0-1"}, + {Type: clusterv1.MachineInternalIP, Address: "10.0.0.1"}, + {Type: clusterv1.MachineExternalIP, Address: "203.0.113.1"}, + {Type: clusterv1.MachineExternalDNS, Address: "node.example.com"}, }). WithPhase("Running"). WithFailureReason(ptr.To(capierrors.MachineStatusError("InvalidConfiguration"))). WithFailureMessage(ptr.To(string("Test failure message"))). WithConditions([]clusterv1.Condition{condition}). Build() @@ - Expect(mapiStatus.Addresses).To(SatisfyAll( - HaveLen(2), - ContainElement(SatisfyAll(HaveField("Type", corev1.NodeInternalIP), HaveField("Address", "10.0.0.1"))), - ContainElement(SatisfyAll(HaveField("Type", corev1.NodeExternalIP), HaveField("Address", "203.0.113.1"))), - )) + Expect(mapiStatus.Addresses).To(SatisfyAll( + HaveLen(3), + ContainElement(SatisfyAll(HaveField("Type", corev1.NodeHostName), HaveField("Address", "ip-10-0-0-1"))), + ContainElement(SatisfyAll(HaveField("Type", corev1.NodeInternalIP), HaveField("Address", "10.0.0.1"))), + ContainElement(SatisfyAll(HaveField("Type", corev1.NodeExternalIP), HaveField("Address", "203.0.113.1"))), + )) + Expect(mapiStatus.Addresses).NotTo(ContainElement(HaveField("Type", corev1.NodeExternalDNS)))Also applies to: 136-150
pkg/conversion/test/fuzz/fuzz.go (2)
298-303: TODO comment needs tracking.The TODO comment indicates that ProviderStatus conversion is not yet implemented. This should be tracked formally.
Would you like me to create a GitHub issue to track the implementation of CAPI InfraMachine to MAPI Machine ProviderStatus conversion?
767-802: Consider extracting address generation logic to reduce duplication.The address fuzzing functions have similar IP generation logic that could be extracted to helper functions for better maintainability.
Consider extracting IP address generation into helper functions:
+func generatePrivateIP(c randfill.Continue) string { + switch c.Int31n(3) { + case 0: + // 10.0.0.0/8 + return fmt.Sprintf("10.%d.%d.%d", + c.Int31n(256), c.Int31n(256), c.Int31n(254)+1) + case 1: + // 172.16.0.0/12 + return fmt.Sprintf("172.%d.%d.%d", + c.Int31n(16)+16, c.Int31n(256), c.Int31n(254)+1) + case 2: + // 192.168.0.0/16 + return fmt.Sprintf("192.168.%d.%d", + c.Int31n(256), c.Int31n(254)+1) + } + return "" +} +func generatePublicIP(c randfill.Continue) string { + return fmt.Sprintf("%d.%d.%d.%d", + c.Int31n(223)+1, // 1-223 (avoid 0.x.x.x and 224+ multicast) + c.Int31n(256), // 0-255 + c.Int31n(256), // 0-255 + c.Int31n(254)+1) // 1-254 (avoid .0 and .255) +} func fuzzMAPIMachineStatusAddress(address *corev1.NodeAddress, c randfill.Continue) { // ... existing switch logic ... case 1: address.Type = corev1.NodeExternalIP - address.Address = fmt.Sprintf("%d.%d.%d.%d", - c.Int31n(223)+1, - c.Int31n(256), - c.Int31n(256), - c.Int31n(254)+1) + address.Address = generatePublicIP(c) case 2: address.Type = corev1.NodeInternalIP - // ... existing IP generation logic ... + address.Address = generatePrivateIP(c)pkg/util/sync.go (1)
266-279: Consider extracting common condition comparison logic.The v1beta1 condition comparison logic is duplicated across CAPI and MAPI versions with only type differences.
Consider using generics to reduce duplication:
// compareConditions compares conditions of type T func compareConditions[T any](a, b []T, getFields func(T) (string, string, string, string, string)) []string { diff := []string{} // Common comparison logic here return diff }pkg/controllers/machinesync/machine_sync_controller.go (2)
606-607: Remove unnecessary unparam lint directive.The
unparamdirective appears to be no longer needed since the function now returns three values including the booleansyncronizationIsProgressing.-//nolint:funlen,unparam +//nolint:funlen func (r *MachineSyncReconciler) createOrUpdateCAPIInfraMachine(ctx context.Context, mapiMachine *machinev1beta1.Machine, infraMachine client.Object, newCAPIInfraMachine client.Object) (ctrl.Result, bool, error) {
809-810: Remove outdated unparam lint directive.Similar to the previous comment, this unparam directive appears unnecessary.
-//nolint:unparam func (r *MachineSyncReconciler) createOrUpdateMAPIMachine(ctx context.Context, existingMAPIMachine *machinev1beta1.Machine, convertedMAPIMachine *machinev1beta1.Machine) (ctrl.Result, error) {pkg/conversion/mapi2capi/machine_test.go (1)
261-271: Add tests for Provisioned, Failed, and Deleting phasesConversion already maps phases — add tests asserting these expectations:
- Provisioned: InfrastructureReady = true; Ready = false; BootstrapReady = false.
- Deleting (phase): BootstrapReady = true; InfrastructureReady = true; Ready = false. v1beta2 "Deleting" condition is driven by DeletionTimestamp (test both phase-only and with DeletionTimestamp); NodeReady is true only if NodeRef is present.
- Failed: Ready, BootstrapReady, InfrastructureReady should all be false.
Suggested location: pkg/conversion/mapi2capi/machine_test.go (near existing Running/Provisioning tests).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
pkg/controllers/machinesetsync/machineset_sync_controller.go(1 hunks)pkg/controllers/machinesync/machine_sync_controller.go(7 hunks)pkg/conversion/capi2mapi/machine.go(5 hunks)pkg/conversion/capi2mapi/machine_test.go(3 hunks)pkg/conversion/mapi2capi/machine.go(3 hunks)pkg/conversion/mapi2capi/machine_test.go(2 hunks)pkg/conversion/test/fuzz/fuzz.go(7 hunks)pkg/util/conditions.go(2 hunks)pkg/util/sync.go(5 hunks)
🔇 Additional comments (15)
pkg/conversion/mapi2capi/machine.go (2)
65-66: Status construction LGTM; verify LastTransitionTime is managed upstream.The conversion sets Condition structs directly; ensure the writer uses conditions.Set(...) when patching to populate LastTransitionTime consistently.
Also applies to: 85-105
198-202: Explicit skips for NodeInfo and clean v1beta2 population LGTM.No further action.
Also applies to: 205-209
pkg/util/conditions.go (2)
116-125: Helper rename and behavior: LGTM.Signature and behavior unchanged; safe migration for callers.
127-160: Condition upsert semantics look correct.
- Updates LTT only on state change or preserves existing when unchanged.
- Uses UTC and second precision.
pkg/conversion/capi2mapi/machine.go (1)
40-63: Status conversion integration: LGTM.Errors are plumbed and status is assigned early; downstream errs are aggregated.
Also applies to: 73-76
pkg/controllers/machinesetsync/machineset_sync_controller.go (1)
1074-1076: Renamed helper usage LGTM.Switch to util.SetMAPICondition is correct and matches the helper rename.
pkg/conversion/mapi2capi/machine_test.go (2)
146-514: Comprehensive test coverage for MAPI to CAPI Machine status conversion.The test suite provides excellent coverage of status conversion scenarios including phase transitions, conditions, addresses, and error states. The test structure follows best practices with clear test names and proper assertions.
441-459: Good test coverage for NodeRef conditional logic.The tests properly verify that NodeReady condition depends on both the phase being "Running" and NodeRef being set, which matches the expected behavior.
pkg/conversion/test/fuzz/fuzz.go (1)
127-136: Status field exclusions are correctly handled for round-trip testing.The exclusion of non-mappable fields (CertificatesExpiryDate, Deletion, NodeInfo) from CAPI to MAPI conversion is appropriate since these fields don't exist in MAPI.
pkg/util/sync.go (2)
84-137: Good implementation of status comparison with v1beta2 support.The function properly handles nil checks for V1Beta2 status before comparison and includes all relevant status fields.
237-282: Efficient condition comparison implementation.The condition comparison functions correctly ignore LastTransitionTime differences and only compare meaningful fields. The early break optimization is good.
pkg/controllers/machinesync/machine_sync_controller.go (4)
712-736: Clean separation of concerns in ensureCAPIMachine.The function follows single responsibility principle by only handling creation logic.
1472-1520: Good implementation of conditional status updates.The function correctly checks for synchronized generation before updating status, preventing race conditions. The use of patch operations with proper error handling is appropriate.
1619-1647: Comprehensive status field updates.The function correctly updates all relevant status fields including v1beta2 conditions with proper nil checks.
1484-1488: SynchronizedGeneration is updated byapplySynchronizedConditionWithPatch—each “true” condition call passes&…Generation(both CAPI→MAPI and MAPI→CAPI flows)—so the generation check inensureCAPIMachineStatusUpdatedis correctly gated.
|
Moving to draft to address the review |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Addressed all review comments. LMK if you are happy with that. TY |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
pkg/conversion/mapi2capi/machine.go (4)
358-397: Address mapping now robust and safeExplicit mapping (including HostName) and skipping unknown types resolves the earlier issue of malformed entries.
399-407: Simple, correct phase mappingUsing ptr.Deref is concise and matches prior suggestion.
409-416: Correct error reason mappingptr.To(capierrors.MachineStatusError(*mapiErrorReason)) is the right conversion.
418-421: Trivial pass-through is fineReturning the pointer directly keeps this helper minimal as suggested.
🧹 Nitpick comments (3)
pkg/conversion/mapi2capi/machine.go (2)
168-186: Precompute derived flags to reduce repeated callsYou call deriveCAPIBootstrapReadyFromMAPI/deriveCAPIInfrastructureReadyFromMAPI multiple times inside closures. Precompute once (e.g., br := …, ir := …) and reuse for Status/Severity/Reason. Improves readability and avoids redundant work.
Also applies to: 187-211, 268-286, 289-306
329-334: Fix typos in comments“We don't ned” → “We don't need”; “tt require” → “it requires”.
pkg/conversion/mapi2capi/machine_test.go (1)
535-565: Add small tests for HostName mapping and nil addressesConsider:
- HostName mapping: corev1.NodeHostName (“Hostname”) should convert to clusterv1.MachineHostName (“HostName”).
- Nil addresses: ensure nil-in → nil-out without errors.
Example snippets you can add:
It("converts Hostname to HostName", func() { m := machinebuilder.Machine(). WithAddresses([]corev1.NodeAddress{{Type: corev1.NodeHostName, Address: "node-1"}}). Build() s, errs := convertMAPIMachineToCAPIMachineStatus(m) Expect(errs).To(BeEmpty()) Expect(s.Addresses).To(ContainElement(SatisfyAll( HaveField("Type", BeEquivalentTo(clusterv1.MachineHostName)), HaveField("Address", Equal("node-1")), ))) }) It("returns nil addresses when input is nil", func() { m := machinebuilder.Machine().Build() m.Status.Addresses = nil s, errs := convertMAPIMachineToCAPIMachineStatus(m) Expect(errs).To(BeEmpty()) Expect(s.Addresses).To(BeNil()) })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
pkg/conversion/mapi2capi/machine.go(4 hunks)pkg/conversion/mapi2capi/machine_test.go(2 hunks)
🔇 Additional comments (2)
pkg/conversion/mapi2capi/machine.go (1)
39-43: Status conversion wired correctlyStatus is computed once, errors are aggregated, and assigned to the CAPI Machine. Looks good.
Also applies to: 71-71
pkg/conversion/mapi2capi/machine_test.go (1)
147-259: Comprehensive status/conditions coverageSolid end-to-end assertions for fields and both v1beta1/v1beta2 conditions. Nice.
|
/lgtm |
|
Scheduling tests matching the |
damdo
force-pushed
the
machine-status-conversion-and-syncing
branch
from
16aeac1 to
737e205
Compare
|
@damdo: This pull request references OCPCLOUD-2995 which is a valid 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. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/util/sync.go (2)
237-257: Condition adds in desired set are ignored; may skip required status updatescompareCAPIV1Beta1Conditions only checks types present in 'a'. If 'b' introduces a new condition, diff stays empty and status update may be skipped.
Apply symmetric comparison to catch additions:
func compareCAPIV1Beta1Conditions(a, b []clusterv1.Condition) []string { diff := []string{} // Compare the conditions one by one. // Ignore the differences in LastTransitionTime. for _, condition := range a { for _, conditionB := range b { if condition.Type == conditionB.Type { if condition.Status != conditionB.Status || condition.Severity != conditionB.Severity || condition.Reason != conditionB.Reason || condition.Message != conditionB.Message { diff = append(diff, deep.Equal(condition, conditionB)...) } break // Break out of the inner loop once we have found a match. } } } + // Also detect additions in b not present in a. + for _, conditionB := range b { + found := false + for _, condition := range a { + if condition.Type == conditionB.Type { + found = true + break + } + } + if !found { + diff = append(diff, "missing condition type in source: "+string(conditionB.Type)) + } + } return diff }
263-282: Same issue for MAPI v1beta1 condition comparisonAdd symmetric check to catch conditions present only in 'b'.
func compareMAPIV1Beta1Conditions(a, b []mapiv1beta1.Condition) []string { diff := []string{} // Compare the conditions one by one. // Ignore the differences in LastTransitionTime. for _, condition := range a { for _, conditionB := range b { if condition.Type == conditionB.Type { if condition.Status != conditionB.Status || condition.Severity != conditionB.Severity || condition.Reason != conditionB.Reason || condition.Message != conditionB.Message { diff = append(diff, deep.Equal(condition, conditionB)...) } break // Break out of the inner loop once we have found a match. } } } + // Also detect additions in b not present in a. + for _, conditionB := range b { + found := false + for _, condition := range a { + if condition.Type == conditionB.Type { + found = true + break + } + } + if !found { + diff = append(diff, "missing condition type in source: "+string(conditionB.Type)) + } + } return diff }
♻️ Duplicate comments (2)
pkg/conversion/mapi2capi/machine.go (2)
399-407: YAGNI: inline phase conversionThis wrapper only calls ptr.Deref; consider inlining at call sites to reduce surface.
-func convertMAPIMachinePhaseToCAPI(mapiPhase *string) string { - // ... - return ptr.Deref(mapiPhase, "") -} +// Inline ptr.Deref(mapiPhase, "") at call sites and drop this helper.
418-421: Trivial passthrough helper can be removedFunction returns its input unmodified; suggest inlining to simplify.
-func convertMAPIMachineErrorMessageToCAPIFailureMessage(mapiErrorMessage *string) *string { - return mapiErrorMessage -} +// Inline the pointer directly at call sites and remove this helper.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (11)
pkg/controllers/machinesetsync/machineset_sync_controller.go(1 hunks)pkg/controllers/machinesync/machine_sync_controller.go(13 hunks)pkg/controllers/machinesync/machine_sync_controller_test.go(5 hunks)pkg/conversion/capi2mapi/machine.go(4 hunks)pkg/conversion/capi2mapi/machine_test.go(2 hunks)pkg/conversion/capi2mapi/machineset.go(1 hunks)pkg/conversion/mapi2capi/machine.go(4 hunks)pkg/conversion/mapi2capi/machine_test.go(2 hunks)pkg/conversion/test/fuzz/fuzz.go(7 hunks)pkg/util/conditions.go(3 hunks)pkg/util/sync.go(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/conversion/capi2mapi/machineset.go
- pkg/conversion/test/fuzz/fuzz.go
- pkg/controllers/machinesetsync/machineset_sync_controller.go
- pkg/conversion/capi2mapi/machine.go
- pkg/conversion/capi2mapi/machine_test.go
🔇 Additional comments (4)
pkg/conversion/mapi2capi/machine.go (1)
372-388: Address conversion: good filtering and diagnosticsUnknown types are skipped and surfaced via field.Invalid; no malformed entries emitted. LGTM.
pkg/conversion/mapi2capi/machine_test.go (1)
535-564: Good coverage for address filtering and error reportingTest correctly asserts we emit a single field.Invalid and drop the unrecognized address.
pkg/util/conditions.go (1)
158-165: LastTransitionTime handling fixedNew conditions now get LTT set (UTC, truncated). Matches intent and prevents churn.
pkg/controllers/machinesync/machine_sync_controller.go (1)
273-282: Nil dereference risk in log pathIf sourceCAPIMachine were nil, existingMAPIMachine may also be nil; dereferencing Name would panic. Log safely without dereferencing.
- if sourceCAPIMachine == nil { - logger.Error(errCAPIMachineNotFound, "machine", existingMAPIMachine.Name) + if sourceCAPIMachine == nil { + logger.Error(errCAPIMachineNotFound, "Cluster API machine is nil")Likely an incorrect or invalid review comment.
|
/assign @JoelSpeed @chrischdi For approve/lgtm |
|
/lgtm |
|
Scheduling tests matching the |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed 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
|
ci/prow/e2e-openstack-capi-techpreview passed the testing phase but failed on gathering/post phase. Hence overriding |
|
@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-openstack-capi-techpreview 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. |
|
ci/prow/e2e-openstack-ovn-techpreview passed but timed out during the gathering/post phase /override ci/prow/e2e-openstack-ovn-techpreview |
|
@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-openstack-ovn-techpreview 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. |
|
ci/prow/e2e-gcp-ovn-techpreview timedout, but I have manually verified the CCAPIO controllers are ok Also the CAPI GCP e2e test passed here. Hence overriding it /override ci/prow/e2e-gcp-ovn-techpreview |
|
@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-gcp-ovn-techpreview 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. |
|
@damdo: 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. |
|
/verified later @sunzhaohua2 To verify the machine status is synced over correctly we also need the infra machine's status to be correctly converted. So it is better to verify everything at the end once both are present. |
openshift-ci-robot
added
verified-later
verified
|
@damdo: This PR has been marked to be verified later by 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-merge-bot
Bot
merged commit eea205c
into
openshift:main
6 participants
This work is the Machine version of #329
It adds Machine status conversion and syncing between MAPI and CAPI.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores