Skip to content

Conversation

@AlexAxenti
Copy link
Contributor

@AlexAxenti AlexAxenti commented Mar 30, 2023

The purpose of this feature is to show the duration it took to update or delete each release when a sync, destroy, or apply command is called. Below are some examples:

Old display of affected releases:

UPDATED RELEASES:
NAME                CHART           VERSION
release-one         charts/test       0.1.0
release-two         charts/test       0.1.0 

DELETED RELEASES:
NAME
release-three
release-four

FAILED RELEASES:
NAME
release-five
release-six

New display of affected releases:

UPDATED RELEASES:
NAME              CHART           VERSION     DURATION
release-one       charts/test     0.1.0             8s
release-two       charts/test     0.1.0            13s

DELETED RELEASES:
NAME              DURATION
release-three           0s
release-four            0s

FAILED RELEASES:
NAME              CHART           VERSION  
release-five      charts/test       0.1.0      
release-six       charts/test       0.1.0  

Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
felipecrs
felipecrs previously approved these changes Mar 31, 2023
Copy link
Contributor

@felipecrs felipecrs left a comment

Choose a reason for hiding this comment

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

For people who uses helmfile with many and complex releases, with several helm level install/uninstall hooks and also helmfile level sync/delete hooks, it would be nice to have a summary at the end with how much time was spent in processing each individual release.

Having this information in a CI loop, for example, allows me to find out which of my releases is the actual bottleneck and deserves more optimization.

ShareX_k8I6zWZIbP

Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
@AlexAxenti AlexAxenti marked this pull request as ready for review March 31, 2023 17:38
@yxxhero
Copy link
Member

yxxhero commented Mar 31, 2023

/cc @mumoshu I think this is a good idea. I saw the code. The logic is straightforward. and almost all changed files are test files.

@yxxhero
Copy link
Member

yxxhero commented Mar 31, 2023

/cc @xiaomudk waiting for your review.

@mumoshu
Copy link
Contributor

mumoshu commented Apr 1, 2023

I'll review this asap! Thanks a lot for your efforts!

xiaomudk
xiaomudk previously approved these changes Apr 14, 2023
@xiaomudk
Copy link
Contributor

@AlexAxenti please fix DCO~
DCO

@yxxhero
Copy link
Member

yxxhero commented Apr 14, 2023

@felipecrs need your help.

@felipecrs
Copy link
Contributor

@felipecrs need your help.

Yeah.. there is not much I can do now unfortunately.

But thanks a lot anyway! Let's wait for @AlexAxenti.

@mumoshu
Copy link
Contributor

mumoshu commented Apr 14, 2023

@xiaomudk @yxxhero Hey! I think @AlexAxenti has already signed-off every commit he added and it would be 8cae3e5 which made DCO check unhappy!

Also, 8cae3e5 broken tests and other references to former "Duration" which is now "duration" so we'd better check this branch out locally and amend the commit to include other fixes, along with a sign-off.

@felipecrs
Copy link
Contributor

felipecrs commented Apr 14, 2023

@AlexAxenti and I can also fix this tomorrow, if you prefer.

Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
@AlexAxenti
Copy link
Contributor Author

Just fixed the errors caused by the incorrect Duration being referenced as well as signed-off the commits, hopefully should be good now.

Signed-off-by: Alexandru Axenti <alex.axenti@gmail.com>
Copy link
Member

@yxxhero yxxhero left a comment

Choose a reason for hiding this comment

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

LGTM

@yxxhero yxxhero merged commit 0012e7e into helmfile:main Apr 15, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants