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

Handle duplicated names in a stage #147

Merged

Conversation

mauromorales
Copy link
Collaborator

@mauromorales mauromorales commented Apr 26, 2024

relates to kairos-io/kairos#2488

description on the issue can be found on the ticket mentioned above

relates to kairos-io/kairos#2488

Signed-off-by: Mauro Morales <contact@mauromorales.com>
@mauromorales mauromorales self-assigned this Apr 26, 2024
Copy link
Collaborator

@Itxaka Itxaka left a comment

Choose a reason for hiding this comment

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

Incredible job Mauro!

prev := ""
for i, st := range currentStages {
name := st.Name
if duplicatedNames {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we actually need to check or can we skip that and just add that index always and call it a day?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Itxaka yeah that was my first approach. I went with this because otherwise if you want to reference a step by name, you have to know in which position it will end. Thankfully there was a test so I saw this problem:

https://github.com/mudler/yip/blob/master/pkg/executor/default_test.go#L226

would need to be referenced as test.0 which would be very confusing, specially if some step gets introduced before the wanted one

@mauromorales mauromorales merged commit 9394813 into mudler:master Apr 26, 2024
3 checks passed
davidcassany pushed a commit to davidcassany/yip that referenced this pull request Jul 3, 2024
kkaempf pushed a commit to rancher/yip that referenced this pull request Jul 3, 2024
* Set the vfat label using the correct flag (mudler#141)

-i expects a hexadecimal value
-n should be used for the filesystem label

https://man7.org/linux/man-pages/man8/mkfs.vfat.8.html

Needed as part of kairos-io/kairos#2281

so that this config:

```
      add_partitions:
        - fsLabel: COS_GRUB
          size: 64
          pLabel: efi
          filesystem: "fat"
```

won't result in this error:

```
Volume ID must be a hexadecimal number
```

Signed-off-by: Dimitris Karakasilis <dimitris@karakasilis.me>
(cherry picked from commit 4ebbc75)

* Reuse existing user id if it exists (mudler#145)

(cherry picked from commit 9484451)

* Handle duplicated names in a stage (mudler#147)

(cherry picked from commit 9394813)

---------

Co-authored-by: Dimitris Karakasilis <jimmykarily@gmail.com>
Co-authored-by: Itxaka <itxaka.garcia@spectrocloud.com>
Co-authored-by: Mauro Morales <contact@mauromorales.com>
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

2 participants