search

metal3-io / cluster-api-provider-metal3

Metal³ integration with https://github.com/kubernetes-sigs/cluster-api
Apache License 2.0
194 stars 87 forks source link

🐛 Remove clearError() function for infrastructure objects #1767

Closed kashifest closed 3 weeks ago

kashifest commented 3 weeks ago

This is a bug and it is a followup of https://github.com/metal3-io/cluster-api-provider-metal3/pull/1765. According to

once failureReason or failureMessage surface on the CAPI object who is referencing the infra object, they cannot be restored anymore. Hence, CAPM3 should not clear them as well.

For the same reason, this PR is removing one of the occurrences of setError so as not to through terminal error while associating BMH but through transient error.
Signed-off-by: Kashif Khan kashif.khan@est.tech

kashifest commented 3 weeks ago

/test metal3-ubuntu-e2e-integration-test-main /test metal3-centos-e2e-integration-test-main

kashifest commented 3 weeks ago

/test ?

metal3-io-bot commented 3 weeks ago

@kashifest: The following commands are available to trigger required jobs:

The following commands are available to trigger optional jobs:

Use /test all to run the following jobs that were automatically triggered:

In response to [this](https://github.com/metal3-io/cluster-api-provider-metal3/pull/1767#issuecomment-2167535967): >/test ? Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
kashifest commented 3 weeks ago

/override metal3-centos-e2e-integration-test-main centos is broken for unrelated reasons /test metal3-ubuntu-e2e-feature-test-main

metal3-io-bot commented 3 weeks ago

@kashifest: Overrode contexts on behalf of kashifest: metal3-centos-e2e-integration-test-main

In response to [this](https://github.com/metal3-io/cluster-api-provider-metal3/pull/1767#issuecomment-2167537595): >/override metal3-centos-e2e-integration-test-main >centos is broken for unrelated reasons >/test metal3-ubuntu-e2e-feature-test-main Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
kashifest commented 3 weeks ago

/override metal3-centos-e2e-integration-test-main

metal3-io-bot commented 3 weeks ago

@kashifest: Overrode contexts on behalf of kashifest: metal3-centos-e2e-integration-test-main

In response to [this](https://github.com/metal3-io/cluster-api-provider-metal3/pull/1767#issuecomment-2167547916): >/override metal3-centos-e2e-integration-test-main Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
adilGhaffarDev commented 3 weeks ago

can we change this SetError to transient error too, that we are doing in Associate : https://github.com/metal3-io/cluster-api-provider-metal3/blob/e827f42e1a94149c789fed5828a924ff0a0c002d/baremetal/metal3machine_manager.go#L391

kashifest commented 3 weeks ago

can we change this SetError to transient error too, that we are doing in Associate :

https://github.com/metal3-io/cluster-api-provider-metal3/blob/e827f42e1a94149c789fed5828a924ff0a0c002d/baremetal/metal3machine_manager.go#L391

Ah ok, I thought we did that already !

adilGhaffarDev commented 3 weeks ago

Ah ok, I thought we did that already !

I only changed errors in SetPause and RemovePause yesterday, didn't want to break anything.

kashifest commented 3 weeks ago

/test metal3-ubuntu-e2e-integration-test-main /override metal3-centos-e2e-integration-test-main /test metal3-ubuntu-e2e-feature-test-main

metal3-io-bot commented 3 weeks ago

@kashifest: Overrode contexts on behalf of kashifest: metal3-centos-e2e-integration-test-main

In response to [this](https://github.com/metal3-io/cluster-api-provider-metal3/pull/1767#issuecomment-2167823353): >/test metal3-ubuntu-e2e-integration-test-main >/override metal3-centos-e2e-integration-test-main >/test metal3-ubuntu-e2e-feature-test-main Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
metal3-io-bot commented 3 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adilGhaffarDev

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: - ~~[OWNERS](https://github.com/metal3-io/cluster-api-provider-metal3/blob/main/OWNERS)~~ [adilGhaffarDev] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment