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.

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

Move error handling for kpt live commands into kpt error resolver #1840

Merged
merged 2 commits into from Apr 30, 2021

Conversation

mortent
Copy link
Contributor

@mortent mortent commented Apr 29, 2021

This combines the new error resolver in kpt with the similar solution in cli-utils. With this change, we use one way to handle errors from both git and live.
This also expands the ErrorResolver somewhat by letting the resolver also determine the exit code for an error.

@frankfarzan
Copy link
Contributor

In general, can you include a sample output in the PR descriptions for UX changes.

@mortent
Copy link
Contributor Author

mortent commented Apr 29, 2021

This doesn't really make any changes to the output, it just consolidates the mechanism we use to resolve errors to user-facing messages.

An example of what the output looks like if applying a set of resources times out:

$ kpt live apply . --reconcile-timeout=2s --output=table
NAMESPACE   RESOURCE                                  ACTION        STATUS      CONDITIONS                                AGE     MESSAGE                                 
            Namespace/bar                             Created       Current     <None>                                    6s      Resource is current                     
            CustomResourceDefinition/traitdefinition  Created       Current     NamesAccepted,Established                 6s      CRD is established                      
            TraitDefinition/manualscalertraits.core.  Created       Current     <None>                                    3s      Resource is current                     
default     Pod/test-pod                              Created       InProgress  Initialized,Ready,ContainersReady,PodSch  3s      Pod is in the Pending phase             
default     Service/nginx                             Created       Current     <None>                                    3s      Service is ready                        
default     Deployment/nginx-deployment               Created       InProgress  Available,Progressing                     3s      Available: 0/2                          
default     └─ ReplicaSet/nginx-deployment-787876777                InProgress  <None>                                    3s      Available: 0/2                          
default        ├─ Pod/nginx-deployment-7878767776-4f                InProgress  Initialized,Ready,ContainersReady,PodSch  3s      Pod is in the Pending phase             
default        └─ Pod/nginx-deployment-7878767776-zl                InProgress  Initialized,Ready,ContainersReady,PodSch  3s      Pod is in the Pending phase             
default     StatefulSet/web                           Created       InProgress  <None>                                    3s      Replicas: 1/2                           
default     └─ Pod/web-0                                            InProgress  <None>                                    3s      Pod is in the Pending phase             
default     CronJob/hello                             Created       Current     <None>                                    3s      Resource is always ready                
default     PodDisruptionBudget/mypdb                 Created       Current     <None>                                    3s      AllowedDisruptions has been computed.   


Error: Timeout after 2 seconds waiting for 3 out of 9 resources to reach condition AllCurrent:
Deployment/nginx-deployment InProgress Available: 0/2
StatefulSet/web InProgress Replicas: 1/2
Pod/test-pod InProgress Pod is in the Pending phase 

Copy link
Contributor

@droot droot left a comment

Choose a reason for hiding this comment

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

Looks good to me. Minor comments related to naming. Will defer to @frankfarzan for any output message related review.

main.go Outdated Show resolved Hide resolved
internal/errors/resolver/resolver.go Outdated Show resolved Hide resolved
@frankfarzan
Copy link
Contributor

This doesn't really make any changes to the output, it just consolidates the mechanism we use to resolve errors to user-facing messages.

An example of what the output looks like if applying a set of resources times out:

$ kpt live apply . --reconcile-timeout=2s --output=table
NAMESPACE   RESOURCE                                  ACTION        STATUS      CONDITIONS                                AGE     MESSAGE                                 
            Namespace/bar                             Created       Current     <None>                                    6s      Resource is current                     
            CustomResourceDefinition/traitdefinition  Created       Current     NamesAccepted,Established                 6s      CRD is established                      
            TraitDefinition/manualscalertraits.core.  Created       Current     <None>                                    3s      Resource is current                     
default     Pod/test-pod                              Created       InProgress  Initialized,Ready,ContainersReady,PodSch  3s      Pod is in the Pending phase             
default     Service/nginx                             Created       Current     <None>                                    3s      Service is ready                        
default     Deployment/nginx-deployment               Created       InProgress  Available,Progressing                     3s      Available: 0/2                          
default     └─ ReplicaSet/nginx-deployment-787876777                InProgress  <None>                                    3s      Available: 0/2                          
default        ├─ Pod/nginx-deployment-7878767776-4f                InProgress  Initialized,Ready,ContainersReady,PodSch  3s      Pod is in the Pending phase             
default        └─ Pod/nginx-deployment-7878767776-zl                InProgress  Initialized,Ready,ContainersReady,PodSch  3s      Pod is in the Pending phase             
default     StatefulSet/web                           Created       InProgress  <None>                                    3s      Replicas: 1/2                           
default     └─ Pod/web-0                                            InProgress  <None>                                    3s      Pod is in the Pending phase             
default     CronJob/hello                             Created       Current     <None>                                    3s      Resource is always ready                
default     PodDisruptionBudget/mypdb                 Created       Current     <None>                                    3s      AllowedDisruptions has been computed.   

remove an extra blank line here

Error: Timeout after 2 seconds waiting for 3 out of 9 resources to reach condition AllCurrent:

use a blank line to improve readability.

Deployment/nginx-deployment InProgress Available: 0/2
StatefulSet/web InProgress Replicas: 1/2
Pod/test-pod InProgress Pod is in the Pending phase

Copy link
Contributor

@frankfarzan frankfarzan left a comment

Choose a reason for hiding this comment

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

nits on UI

@mortent mortent merged commit 6219473 into kptdev:next Apr 30, 2021
frankfarzan pushed a commit to frankfarzan/kpt that referenced this pull request Jun 3, 2021
…tdev#1840)

* Move error handling for kpt live commands into kpt error resolver

* Addressed comments
@mortent mortent deleted the CleanUpErrorResolvers branch June 11, 2021 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants