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

Expand last partition after creating the partitions #96

Merged
merged 3 commits into from
Jun 30, 2023

Conversation

jimmykarily
Copy link
Collaborator

@jimmykarily
Copy link
Collaborator Author

the broken job is also broken on master.

@jimmykarily jimmykarily self-assigned this Jun 8, 2023
@Itxaka
Copy link
Collaborator

Itxaka commented Jun 8, 2023

Im not sure if its enough with this. AFAIK the problem was that this plugin on yip does not support GPT partitions, but I dont see any changes to those.

Maybe it was wrong and it does support GPT partitions already?

@jimmykarily
Copy link
Collaborator Author

jimmykarily commented Jun 8, 2023

I don't know. This is not supposed to fix everything. Only porting this specific fix to yip. In the meantime. I simply switched to this plugin in immucore and it expands the partition just fine (in the context of this: kairos-io/kairos#1448).

My plan is to have a look at what the other 2 implementations did (kairos-agent and immucore) and try to predict what could go wrong. I'm afraid there won't be any broken tests to help.

@jimmykarily
Copy link
Collaborator Author

One big difference is that the immucore implementation is using parted while the yip one is using sgdisk to create the partitions.

@jimmykarily
Copy link
Collaborator Author

parted, fdisk and gdisk/sgdisk , they all support GPT nowadays: https://www.rodsbooks.com/gdisk/whygdisk.html

Unless we are not using it to do so.

@jimmykarily
Copy link
Collaborator Author

I switched both the kairos-agent and immucore to the yip implementation and after installation I get this:

localhost:/home/kairos # parted /dev/vda print
Model: Virtio Block Device (virtblk)
Disk /dev/vda: 64.4GB
Sector size (logical/physical): 512B/512B
Partition Table: gpt
Disk Flags: 

Number  Start   End     Size    File system  Name        Flags
 1      1049kB  2097kB  1049kB               bios        bios_grub
 2      2097kB  69.2MB  67.1MB  ext4         oem
 3      69.2MB  8659MB  8590MB  ext4         recovery
 4      8659MB  24.8GB  16.1GB  ext4         state
 5      24.8GB  64.4GB  39.7GB  ext4         persistent

I guess Partition Table: gpt proves GPT works.

@Itxaka
Copy link
Collaborator

Itxaka commented Jun 9, 2023

noiiiice!

@Itxaka
Copy link
Collaborator

Itxaka commented Jun 9, 2023

FYI @jimmykarily you got fixes for the tests in this PR: https://github.com/mudler/yip/pull/95/files

As long as you ignore the changes in the command.go file, the rest of the changes should fix the tests. Do you want them in this patch or want me to send a different PR with just those fixes? Im not gonna merge the linked PR as Im not happy with it but the test fixes would be good to have.

@jimmykarily
Copy link
Collaborator Author

I think a separate PR which we merge and I rebase is the best option, if you have time to prepare that. Otherwise I can try that.

@Itxaka
Copy link
Collaborator

Itxaka commented Jun 9, 2023

I think a separate PR which we merge and I rebase is the best option, if you have time to prepare that. Otherwise I can try that.

yep, give me 5 and I'll send it

@Itxaka
Copy link
Collaborator

Itxaka commented Jun 9, 2023

oh @jimmykarily seems like the only fix is this patch as gox is no longer used:

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 4669bc4..fe0270a 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -13,8 +13,6 @@ jobs:
         uses: actions/checkout@v3
       - name: Test
         run: make test
-      - name: Build
-        run: make deps gox-build
 
   release:
     name: Release

My PR doesnt hit this because it comes from a fork, while yours is run from a branch in the main repo.

Do we want a full PR for this? Is only removing 2 lines...

@jimmykarily
Copy link
Collaborator Author

Just push a commit here, it's ok

pkg/plugins/layout.go Outdated Show resolved Hide resolved
@mudler
Copy link
Owner

mudler commented Jun 20, 2023

@jimmykarily , the partitioner plugin @Itxaka is refering to is https://github.com/kairos-io/kairos-agent/blob/main/pkg/cloudinit/layout_plugin.go - do we need also to backport the patch over there?

Copy link
Owner

@mudler mudler left a comment

Choose a reason for hiding this comment

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

looking good here!

@jimmykarily
Copy link
Collaborator Author

I'm testing this here: kairos-io/kairos#1497 I need to see if this can be used both in kairos-agent and immucore. Switching to this implementation and running the e2e tests is a good way to know.

@mudler
Copy link
Owner

mudler commented Jun 20, 2023

@jimmykarily , the partitioner plugin @Itxaka is refering to is https://github.com/kairos-io/kairos-agent/blob/main/pkg/cloudinit/layout_plugin.go - do we need also to backport the patch over there?

scratch that, saw the PR, I'm still confused - this used to benecessary for GPT.. because was using sgdisk instead of parted?

@jimmykarily
Copy link
Collaborator Author

@jimmykarily , the partitioner plugin @Itxaka is refering to is https://github.com/kairos-io/kairos-agent/blob/main/pkg/cloudinit/layout_plugin.go - do we need also to backport the patch over there?

scratch that, saw the PR, I'm still confused - this used to benecessary for GPT.. because was using sgdisk instead of parted?

Check my 2 comments here: #96 (comment)

@jimmykarily jimmykarily force-pushed the 1448-expand-partitions-after-creation branch from 7066ac1 to 4758d80 Compare June 30, 2023 09:03
@jimmykarily jimmykarily merged commit 8429ac0 into master Jun 30, 2023
@jimmykarily jimmykarily deleted the 1448-expand-partitions-after-creation branch June 30, 2023 09:05
jimmykarily added a commit to kairos-io/kairos-agent that referenced this pull request Jun 30, 2023
to get this fix: mudler/yip#96

Signed-off-by: Dimitris Karakasilis <[email protected]>
jimmykarily pushed a commit to kairos-io/immucore that referenced this pull request Jun 30, 2023
to get this fix: mudler/yip#96

and stop "replacing" in go.mod.

We are not 100% sure what the replacement was there for but we'll test
and see.

Also remove replace of elemental-cli which was there only for debugging

Signed-off-by: Dimitris Karakasilis <[email protected]>
jimmykarily added a commit to kairos-io/kairos-agent that referenced this pull request Jun 30, 2023
to get this fix: mudler/yip#96

Signed-off-by: Dimitris Karakasilis <[email protected]>
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.

3 participants