Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

Return error to reconciler when process manifest fails #664

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

irisdingbj
Copy link
Member

@irisdingbj irisdingbj requested a review from a team as a code owner December 5, 2019 09:47
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Dec 5, 2019
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 5, 2019
Copy link
Member

@morvencao morvencao left a comment

Choose a reason for hiding this comment

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

not sure if this makes sense.
Once these errors are returned to the main reconcile loop, the controller will re-trigger new reconcile loop.
However, some kinds of manifest process errors will emerge, no matter how many times the reconcile loop are triggerred, in these cases, we'd better keep the process in the ICP status.

@@ -115,7 +116,11 @@ func (h *HelmReconciler) Reconcile() error {
errs = util.AppendErr(errs, h.customizer.Listener().EndPrune())

errs = util.AppendErr(errs, h.customizer.Listener().EndReconcile(h.instance, status))

for _, v := range status.Status {
if v.Error != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to check against the enum status field (!=ERROR).

@istio-testing
Copy link

@irisdingbj: PR needs rebase.

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.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. needs-rebase Indicates a PR needs to be rebased before being merged size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants