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

Fix reset #565

Merged
merged 4 commits into from
Sep 27, 2024
Merged

Fix reset #565

merged 4 commits into from
Sep 27, 2024

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented Sep 27, 2024

Seems like witht he older ghw, we were not receiving all the proper partitions and mountpoints and such, so on reset we could blindily try to unmount all partitions and call it a day.

But now we get info about all of them, so doing so will even unmount the current recovery partition, where the reset source is at, thus failing the reset copy.

This patch fixes it by instead of going crazy, making sure we have an order mount/unmount of the proper places at the proper times and we restore everything as is once we are done with it

Seems like witht he older ghw, we were not receiving all the proper
partitions and mountpoints and such, so on reset we could blindily try
to unmount all partitions and call it a day.

But now we get info about all of them, so doing so will even unmount the
current recovery partition, where the reset source is at, thus failing
the reset copy.

This patch fixes it by instead of going crazy, making sure we have an
order mount/unmount of the proper places at the proper times and we
restore everything as is once we are done with it

Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka requested a review from a team September 27, 2024 13:29
@jimmykarily
Copy link
Contributor

jimmykarily commented Sep 27, 2024

These errors would have helped us fixing it. Can we print those errors too?

diff --git a/pkg/utils/common.go b/pkg/utils/common.go
index abc11b5..9262fcb 100644
--- a/pkg/utils/common.go
+++ b/pkg/utils/common.go
@@ -106,14 +106,17 @@ func ConcatFiles(fs v1.FS, sources []string, target string) (err error) {
        for _, source := range sources {
                sourceFile, err := fs.Open(source)
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
                _, err = io.Copy(targetFile, sourceFile)
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
                err = sourceFile.Close()
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
        }

using some logger preferably

@jimmykarily
Copy link
Contributor

These errors would have helped us fixing it. Can we print those errors too?

diff --git a/pkg/utils/common.go b/pkg/utils/common.go
index abc11b5..9262fcb 100644
--- a/pkg/utils/common.go
+++ b/pkg/utils/common.go
@@ -106,14 +106,17 @@ func ConcatFiles(fs v1.FS, sources []string, target string) (err error) {
        for _, source := range sources {
                sourceFile, err := fs.Open(source)
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
                _, err = io.Copy(targetFile, sourceFile)
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
                err = sourceFile.Close()
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
        }

using some logger preferably

actually, just return the error (wrapped or not).

@Itxaka
Copy link
Member Author

Itxaka commented Sep 27, 2024

These errors would have helped us fixing it. Can we print those errors too?

diff --git a/pkg/utils/common.go b/pkg/utils/common.go
index abc11b5..9262fcb 100644
--- a/pkg/utils/common.go
+++ b/pkg/utils/common.go
@@ -106,14 +106,17 @@ func ConcatFiles(fs v1.FS, sources []string, target string) (err error) {
        for _, source := range sources {
                sourceFile, err := fs.Open(source)
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
                _, err = io.Copy(targetFile, sourceFile)
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
                err = sourceFile.Close()
                if err != nil {
+                       fmt.Printf("err.Error() = %+v\n", err.Error())
                        break
                }
        }

using some logger preferably

actually, just return the error (wrapped or not).

tbh, I would like to do this in a different PR if possible> As it can introduce new errors and I want this to fix the actual reset issue before breaking it further lol

Signed-off-by: Itxaka <itxaka@kairos.io>
As we were not providing any mountpoints for our fake partitions, reset
test failed to work

Signed-off-by: Itxaka <itxaka@kairos.io>
Signed-off-by: Itxaka <itxaka@kairos.io>
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 49.80%. Comparing base (84d87b3) to head (efa3762).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/action/reset.go 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
+ Coverage   49.73%   49.80%   +0.06%     
==========================================
  Files          48       48              
  Lines        4588     4582       -6     
==========================================
  Hits         2282     2282              
+ Misses       2029     2026       -3     
+ Partials      277      274       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Itxaka Itxaka merged commit 0f85a2c into main Sep 27, 2024
14 checks passed
@Itxaka Itxaka deleted the fix_reset branch September 27, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants