search

operator-framework / operator-controller

A new and improved management framework for extending Kubernetes with Operators
https://operator-framework.github.io/operator-controller/
Apache License 2.0
61 stars 53 forks source link

:sparkles: mask helm conflict errors #1016

Closed perdasilva closed 2 months ago

perdasilva commented 3 months ago

Description

As part of #736 we'd like to mark helm errors when an installation fails due to conflict. This PR wraps the ActionClient interface we use to talk to helm and processes the errors returned give a set of programatic rules. So far, only the install conflict error translator is modeled

Reviewer Checklist

netlify[bot] commented 3 months ago

Deploy Preview for olmv1 ready!

Name Link
Latest commit d78e9a96a5f438c02f39e2514ed990eb7b3002c9
Latest deploy log https://app.netlify.com/sites/olmv1/deploys/6696723a2fe9700008789218
Deploy Preview https://deploy-preview-1016--olmv1.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 93.65079% with 4 lines in your changes missing coverage. Please review.

Project coverage is 73.29%. Comparing base (422a740) to head (d78e9a9). Report is 2 commits behind head on main.

Files Patch % Lines
internal/action/error/errors.go 92.00% 2 Missing :warning:
internal/action/helm.go 94.59% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1016 +/- ## ========================================== + Coverage 72.63% 73.29% +0.66% ========================================== Files 28 30 +2 Lines 1889 1951 +62 ========================================== + Hits 1372 1430 +58 - Misses 381 384 +3 - Partials 136 137 +1 ``` | [Flag](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1016/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | Coverage Δ | | |---|---|---| | [e2e](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1016/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `54.32% <58.73%> (+0.13%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1016/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `46.64% <84.12%> (+1.27%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

LalatenduMohanty commented 3 months ago

@perdasilva can you please add the results (logging) before and after the PR here. Which will help me better understand the difference in user experience.

perdasilva commented 3 months ago

@perdasilva can you please add the results (logging) before and after the PR here. Which will help me better understand the difference in user experience.

Went from:

CustomResourceDefinition "applications.argoproj.io" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "clusterextension-test": current value is "clusterextension-sample"; annotation validation error: key "meta.helm.sh/release-namespace" must equal "test": current value is "default"

To:

CustomResourceDefinition 'applications.argoproj.io' already exists in namespace ''
LalatenduMohanty commented 3 months ago

CustomResourceDefinition 'applications.argoproj.io' already exists in namespace ''

@joelanford mentioned that this logging is also not right because it does not the point right root cause to the user.

perdasilva commented 3 months ago

CustomResourceDefinition 'applications.argoproj.io' already exists in namespace ''

@joelanford mentioned that this logging is also not right because it does not the point right root cause to the user.

it would be good to have an example..? or alternatively, here is the line. Please feel free to suggest a change