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

🌱 Deprecate obsolete errors pkg #10798

Merged
merged 1 commit into from
Sep 4, 2024
Merged

🌱 Deprecate obsolete errors pkg #10798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/book/src/SUMMARY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
- [v1.5 to v1.6](./developer/providers/migrations/v1.5-to-v1.6.md)
- [v1.6 to v1.7](./developer/providers/migrations/v1.6-to-v1.7.md)
- [v1.7 to v1.8](./developer/providers/migrations/v1.7-to-v1.8.md)
- [v1.8 to v1.9](./developer/providers/migrations/v1.8-to-v1.9.md)
- [Provider contracts](./developer/providers/contracts.md)
- [Cluster Infrastructure](./developer/providers/cluster-infrastructure.md)
- [Machine Infrastructure](./developer/providers/machine-infrastructure.md)
Expand Down
22 changes: 22 additions & 0 deletions docs/book/src/developer/providers/migrations/v1.8-to-v1.9.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Cluster API v1.8 compared to v1.9

This document provides an overview over relevant changes between Cluster API v1.8 and v1.9 for
maintainers of providers and consumers of our Go API.

## Go version

- The Go version used by Cluster API is Go 1.22.x

## Changes by Kind

### Deprecation

### Removals

### API Changes

### Other

### Suggested changes for providers

- The Errors package was created when capi provider implementation was running as machineActuators that needed to vendor core capi to function. There is no usage recommendations today and its value is questionable since we moved to CRDs that inter-operate mostly via conditions. Instead we plan to drop the dedicated semantic for terminal failure and keep improving Machine lifecycle signal through conditions. Therefore the Errors package [has been deprecated in v1.8](https://github.com/kubernetes-sigs/cluster-api/issues/10784). It's recommented to remove any usage of the currently exported variables.
Copy link
Member

Choose a reason for hiding this comment

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

The Errors package was created when capi provider implementation was running as machineActuators that needed to vendor core capi to function. There is no usage recommendations today

Just to avoid misunderstandings (and to ensure we're talking about the same thing)

MachineStatusError & ClusterStatusError are types used in our API and they are both used to read the corresponding fields from InfrastructureCluster & InfrastructureMachine (e.g.:

cluster-api/internal/controllers/machine/machine_controller_phases.go

Lines 105 to 111 in 4714c48

failureReason, failureMessage, err := external.FailuresFrom(obj)
if err != nil {
return external.ReconcileOutput{}, err
}
if failureReason != "" {
machineStatusError := capierrors.MachineStatusError(failureReason)
m.Status.FailureReason = &machineStatusError
)

So as of right now, they are even part of our contract (vs. "no usage recommendation").
But this will be deprecated with #10897 and then removed with v1beta2.

Correct?

Copy link
Member

Choose a reason for hiding this comment

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

(cc @fabriziopandini)

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what I'm proposing yes. The whole package mostly got obsolete and unused when we move to CRDs, conditions and continuous reconciliation flow vs create/update. An even more as we move towards dropping existing terminal failure signal in favour of fleshed out conditions now.
If we need to have specific machine error types for v1beta2 I think we should redefine then when the time comes, but those will most likely come as conditions reasons. My intent with this PR is mainly to avoid confusion and perpetuating a dead end pattern e.g. https://github.com/kubernetes-sigs/cluster-api/pull/10360/files

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good!

Copy link
Member

Choose a reason for hiding this comment

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

+1

2 changes: 2 additions & 0 deletions errors/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ limitations under the License.
*/

// Package errors makes a set of error message handlers available for use by Cluster API Providers.
//
// Deprecated: This package will be removed in one of the next releases.
package errors
Loading