[SURE-9137] Add template errors to bundle and gitrepo status #3114
Conversation
p-se
force-pushed
the
SURE-9137
branch
4 times, most recently
from
d234de6 to
3c04dfc
Compare
|
Where are Bundle errors displayed in the UI ? |
Good point, the Bundle is not something we like to advertise to users. We're treating it more like an internal resource in the Rancher UI. It would be in That said, the same error will be in the GitRepo.
|
| if c.Type == string(fleet.Ready) && c.Status == v1.ConditionFalse { | ||
| condition = c | ||
| found = true | ||
| break bundles |
There was a problem hiding this comment.
It looks like this will only propagate the first bundle rendering error to the gitrepo status, if multiple such errors exist over multiple bundles.
Perhaps something to clarify in documentation.
There was a problem hiding this comment.
Yes, that's true. That is something that we might want to change at some point, but I'd need to think more about it, since there's another possible optimization. In the meantime, I'm thinking of adding some documentation to our docs, too, since the errors we start showing in Bundle and GitRepo won't be found in Cluster or ClusterGroup.
e2e/single-cluster/status_test.go
Outdated
| }) | ||
| }) | ||
|
|
||
| // getGitRepoStatus retrieves the status of the gitrepo with the provided name. |
There was a problem hiding this comment.
| // getGitRepoStatus retrieves the status of the gitrepo with the provided name. | |
| // getBundleStatus retrieves the status of the bundle with the provided name. |
There was a problem hiding this comment.
There were 2 suggested changes on this line ;)
| @@ -107,3 +117,175 @@ var _ = Describe("Checks status updates happen for a simple deployment", Ordered | |||
| }) | |||
| }) | |||
| }) | |||
|
|
|||
| var _ = Describe("Checks that template errors are shown in bundles and gitrepos", Ordered, Label("infra-setup"), func() { | |||
There was a problem hiding this comment.
What prevents these tests from being integration tests instead?
There was a problem hiding this comment.
I'm not actually sure that there is something that would prevent that, I was just worried about the complexity of having to work with several controllers in integration tests.
There was a problem hiding this comment.
We already support multiple reconcilers in integration tests ;)
p-se
force-pushed
the
SURE-9137
branch
3 times, most recently
from
2edaa8e to
064833e
Compare
p-se
changed the title
When fleet.yaml is evaluated for targeting, errors can occur due to missing values, like for instance, if a cluster has a missing label that is used in the fleet.yaml file.
Those kind of errors would previously only be visible in logs. This PR introduces error messages in the bundle, more specifically in the Ready condition of the Bundle status, when errors occur on rendering the templates for targeting. Subsequently the error message is being copied from the Bundle to the GitRepo. It will not show up in Clusters or ClusterGroups.
It is new that Bundles will have their own errors. Errors are commonly created in BundleDeployments and are aggregated in the status of Bundle. Now, additionally, the bundle itself may have errors that result in a status change. Those errors will take precedence over any errors that may already exist and effectively overwrite the Ready status condition, should they occur.
Overwriting errors coming from BundleDeployments in Bundle will not happen if the Bundle never created a BundleDeployment that has an error, but will happen if BundleDeployments have successfully been created from the Bundle. If then a cluster is added that has a missing label or a cluster is edited so that a label value required for templating is removed, the error will be shown in the Bundle, not those errors that might be present in the BundleDeployment resources. The errors present in the BundleDeployment will not be changed at all, so that there is a possibility of retrieving both at the same time.