pkg/errors is no longer maintained and archived

[sbueringer]

To be honest, I'm not sure what we want to do or if we want to do something, but just found out that pkg/errors is archived.

See: pkg/errors#245

/kind bug
/area dependency
[One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels]

[DiptoChakrabarty]

There are two options which I think can be done

• keep things as they are
• replace with native golang error wrapping which can be handled by the fmt and errors package

there might be more but these come to my mind

there are few issues regarding this in other repos kubernetes/release#2549

[sbueringer]

I have to think about it a bit more but the approach in kubernetes/release#2549 sounds like a reasonable option and obviously consistent with upstream Kubernetes.

[oscr]

@sbueringer What are your thoughts on this? Is this something we can start working on, but wait with prs until 1.2 is released?

[sbueringer]

I think we need more discussion before we should do anything.

I would especially like to hear the opinions of other maintainers on this as it's a pretty fundamental change

(@enxebre @CecileRobertMichon @fabriziopandini @vincepri)

[fabriziopandini]

I would align to whatever k/k does
@neolit123 any info WRT to ^^

[neolit123]

the Wrap(f) gives formatting consistency and avoids leakage of text: ... vs text : ... formatting diffs. stdlib has no answer.

k/k is not taking action for kubeadm for the time being, although there was a ticket somewhere.
unless k/k/kubeadm is forced to switch we can continue vendoring it.

[fabriziopandini]

/triage accepted

Let's keep this around to monitor the situation, but I will aling to k/k and not take action for the time being

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[oscr]

/remove-lifecycle stale

[fabriziopandini]

/lifecycle frozen

[fabriziopandini]

(doing some cleanup on old issues without updates)
/close
unfortunately, no one is picking up the task and there is no real actionable item. the thread will remain available for future reference

[k8s-ci-robot]

@fabriziopandini: Closing this issue.

In response to this:

(doing some cleanup on old issues without updates)
/close
unfortunately, no one is picking up the task. the thread will remain available for future reference

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/test-infra repository.

[sbueringer]

/reopen

Given that I see continued comments from maintainers if we should get rid of pkg/errors

[k8s-ci-robot]

@sbueringer: Reopened this issue.

In response to this:

/reopen

Given that I see continued comments from maintainers if we should get rid of pkg/errors

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/test-infra repository.

[sbueringer]

The problem with just directly replacing it through Go SDK '%w' is that we lose information. Most visible in logs.

Example:

	err1 := fmt.Errorf("err1")
	err2 := fmt.Errorf("err2: %w", err1)
	log.Error(err2, "Go SDK error wrapping")

	err1 = pkgerrors.New("err1")
	err2 = pkgerrors.Wrapf(err1, "err2")
	log.Error(err2, "pkgerrors wrapping")

Resulting logs

{"ts":1706257281192.585,"caller":"cluster-api/main.go:252","msg":"Go SDK error wrapping","err":"err2: err1"}
{"ts":1706257281192.6028,"caller":"cluster-api/main.go:256","msg":"pkgerrors wrapping","err":"err2: err1","errVerbose":"err1\nmain.main\n\t/Users/buringerst/code/src/sigs.k8s.io/cluster-api/main.go:254\nruntime.main\n\t/Users/buringerst/go_sdks/go1.21.5/src/runtime/proc.go:267\nruntime.goexit\n\t/Users/buringerst/go_sdks/go1.21.5/src/runtime/asm_arm64.s:1197\nerr2\nmain.main\n\t/Users/buringerst/code/src/sigs.k8s.io/cluster-api/main.go:255\nruntime.main\n\t/Users/buringerst/go_sdks/go1.21.5/src/runtime/proc.go:267\nruntime.goexit\n\t/Users/buringerst/go_sdks/go1.21.5/src/runtime/asm_arm64.s:1197"}

Note: Full error stack traces are only implemented with JSON format not with text. This is because JSON logging in component-base is implemented with zap and zap automatically adds errVerbose if err implements fmt.Formatter (xref: https://github.com/uber-go/zap/blob/master/zapcore/error.go#L70-L77)

As can be seen, pkg/errors preserves the stack trace of where the error occurs which can vastly simplify debugging.

I think we have the following options:

• lose information and move to Go SDK wrapping
• implement our own util package which just preserves the stacktrace and is otherwise a 1:1 replacement for pkg/errors (not sure how much code we're talking about)
• keep pkg/errors (not great, given that it's unmaintained)

If we have consensus replacing it could potentially be pretty straightforward using Intellij's "Structural Search and Replace feature" (https://www.jetbrains.com/help/idea/structural-search-and-replace.html#to_search_structurally).

[neolit123]

keep pkg/errors (not great, given that it's unmaintained)

if it's no longer maintainer, but it has no critical bugs, then there is no problem with using it?

implement our own util package which just preserves the stacktrace and is otherwise a 1:1 replacement for pkg/errors (not sure how much code we're talking about)

maybe we can try to add it in component-base?
https://github.com/kubernetes/component-base
needs sig-arch approval, but makes more sense.

or kubernetes/kubeadm/errors?
@fabriziopandini

lose information and move to Go SDK wrapping

and lose formatting, since sometimes err2 := fmt.Errorf("err2: %w", err1) can be missed during review.

[sbueringer]

if it's no longer maintainer, but it has no critical bugs, then there is no problem with using it?

Good point. I think there is no reason why we would have to migrate away now.

I mostly reopened this because it was brought up a few times on PRs and this issue seems like a better place for discussion.

[fabriziopandini]

I don't think that as a CAPI community w should/we have enough resources to maintain an error package
I would simply align at what k/k does.

[neolit123]

I would simply align at what k/k does.

in k/k there was a tracking issue to drop all usage, but for cmd/kubeadm we requested an exception.

[fabriziopandini]

/kind cleanup
/important long-term

[fabriziopandini]

/priority important-longterm