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.

Sign up for GitHub

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

Jump to bottom

kapp app-group deploy should order based on folder names #755

Closed
Baune8D opened this issue May 16, 2023 · 4 comments · Fixed by #850
Closed

kapp app-group deploy should order based on folder names #755

Baune8D opened this issue May 16, 2023 · 4 comments · Fixed by #850
Assignees
@rjtch
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed good first issue An issue that will be a good candidate for a new contributor

Comments

@Baune8D
Copy link

Baune8D commented May 16, 2023

What steps did you take:
Deploy any app using kapp app-group deploy

What happened:
App deployment order is randomized on every execution.

What did you expect:
App deployment order should be alphabetically based on folder names.

Anything else you would like to add:
When reading directory using ioutil.ReadDir the ordering is correct, but when assigning to the result map here:

kapp/pkg/kapp/cmd/appgroup/deploy.go

Line 137 in df3cfc8

result[app.Name] = app
ordering gets randomized.

Environment:

  • kapp version (use kapp --version): 0.56.0
  • OS (e.g. from /etc/os-release): macOS Ventura
  • Kubernetes version (use kubectl version) 1.25.7

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.
@Baune8D Baune8D added bug This issue describes a defect or unexpected behavior carvel triage This issue has not yet been reviewed for validity labels May 16, 2023
@praveenrewar praveenrewar added good first issue An issue that will be a good candidate for a new contributor carvel accepted This issue should be considered for future work and that the triage process has been completed and removed carvel triage This issue has not yet been reviewed for validity labels May 17, 2023
@rjtch
Copy link
Contributor

rjtch commented Sep 30, 2023

hey @Baune8D i would like to work on this task

@praveenrewar
Copy link
Member

Thank you for showing interest in the fixing this @rjtch! I am assigning the issue to you, let me know if you need any help with it.

@rjtch
Copy link
Contributor

rjtch commented Oct 8, 2023

Hey @praveenrewar I would like to use a this library https://github.com/wk8/go-ordered-map to fix it. Is it okay or do you have any recommendations?

rjtch pushed a commit to rjtch/kapp that referenced this issue Oct 8, 2023
kapp app-group deploy should order based on folder names carvel-dev#755
7af7941
@praveenrewar
Copy link
Member

Hey @rjtch! I noticed that you are using a slice in your PR, which is what I would have recommended as well 🚀

rjtch pushed a commit to rjtch/kapp that referenced this issue Oct 15, 2023
fix(carvel-dev#755) - removed map and used slice instead for sorting …
c21ac9b 
…apps
rjtch pushed a commit to rjtch/kapp that referenced this issue Oct 20, 2023
@rjtch
fix(carvel-dev#755) - removed map and used slice instead for sorting …
b922800 
…apps

Signed-off-by: rjtch <[email protected]>
rjtch pushed a commit to rjtch/kapp that referenced this issue Oct 20, 2023
@rjtch
fix(carvel-dev#755) - removed map and used slice instead for sorting …
f6d40a5 
…apps

Signed-off-by: rjtch <[email protected]>
rjtch pushed a commit to rjtch/kapp that referenced this issue Oct 31, 2023
@rjtch
kapp app-group deploy should order based on folder names carvel-dev#755
7fb09d8 
Signed-off-by: rjtch <[email protected]>
rjtch pushed a commit to rjtch/kapp that referenced this issue Oct 31, 2023
@rjtch
fix(carvel-dev#755) - removed map and used slice instead for sorting …
c78ece2 
…apps

Signed-off-by: rjtch <[email protected]>
@praveenrewar praveenrewar mentioned this issue Nov 4, 2023
Merged
5 tasks
praveenrewar pushed a commit that referenced this issue Nov 4, 2023
@rjtch
kapp app-group deploy should order based on folder names (#850)
39761d2 
* kapp app-group deploy should order based on folder names #755

Signed-off-by: rjtch <[email protected]>

* fix(#755) - removed map and used slice instead for sorting apps

Signed-off-by: rjtch <[email protected]>

* feat/755 - removed unecessary printf

Signed-off-by: rjtch <[email protected]>

* feat/755 - removed unecessary signoff

Signed-off-by: rjtch <[email protected]>

* signoff error

Signed-off-by: rjtch <[email protected]>

* signoff error

Signed-off-by: rjtch <[email protected]>

* tried to fix CDo

Signed-off-by: rjtch <[email protected]>

* signed-off last request

Signed-off-by: rjtch <[email protected]>

* signoff-last message and fixed unsigned commits

Signed-off-by: rjtch <[email protected]>

* get back deleted changed

Signed-off-by: rjtch <[email protected]>

* removed unnessary printf

Signed-off-by: rjtch <[email protected]>

---------

Signed-off-by: rjtch <[email protected]>
Co-authored-by: Hergy Fongue <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rjtch rjtch

Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed good first issue An issue that will be a good candidate for a new contributor
Projects
Carvel
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kapp app-group deploy should order based on folder names rjtch/kapp
3 participants
@praveenrewar @rjtch @Baune8D