Set Kopia IgnoreUnknownTypes in ErrorHandlingPolicy to True #5786

qiuming-best · 2023-01-19T09:36:34Z

Set Kopia IgnoreUnknownTypes in ErrorHandlingPolicy to True for ignoring backup unknown or unsupported file type

Signed-off-by: Ming mqiu@vmware.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)
#5684

Please indicate you've done the following:

Accepted the DCO. Commits without the DCO will delay acceptance.
Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
Updated the corresponding documentation in site/content/docs/main.

Lyndon-Li · 2023-01-19T10:25:14Z

pkg/uploader/kopia/snapshot.go

@@ -80,6 +85,9 @@ func setupDefaultPolicy(ctx context.Context, rep repo.RepositoryWriter, sourceIn
 		SchedulingPolicy: policy.SchedulingPolicy{
 			Manual: true,
 		},
+		ErrorHandlingPolicy: policy.ErrorHandlingPolicy{
+			IgnoreUnknownTypes: newOptionalBool(true),


Ignoring errors arbitrarily is dangerous, we cannot set it to true by default.
If we want to support to ignore unknown errors, the option should be disabled by default and we only enable it from a users' option.
Therefore, we have to create a user interface for this feature.

Finally, I think this IgnoreUnknownTypes option is not Must Have. As long as we can capture the error message and report to users(as what we do for ErrorHandlingPolicy) it is good enough.

In fact, the unknown type of files is not regular files at the filesystem level, including setuid/gid, hardlinks, mount points, and UNIX socket. both of them are not usable after restore even if Kopia could backup and users also won't expect these could work. so it's not dangerous if we could show the ignored files as a warning in the backup CR maybe that is enough.

Checked Kopia code further, this IgnoreUnknownTypes option is checked for the file system entries that are not recognized by Kopia uploader. Kopia uploader will fail to back up these entries inevitably and IgnoreUnknownTypes is only to control whether the uploader error out or ignore and process.
However, it doesn't mean these entries are not useful, after all, we even don't know what they are exactly (Kopia at present supports dirs, files and symlinks only, other entry types all go to the UnknownTypes, a.k.a., ErryEntries). Instead, it only means Kopia uploader doesn't support them, but since we are using Kopia uploader, we don't have any way to support them if the uploader doesn't support.

For Kopia itself, it by default sets IgnoreUnknownTypes to true and gives users options to modify it. For Velero:

If we set it to false, we will face some embarrassment that Kopia util doesn't fail itself but Velero fails

If we set it to true, as mentioned above, the unknown entries are still meaningful to users applications, we will also face the embarrassment that when users really want to fail the backup on these unknown entries, Kopia util can but Velero cannot

Therefore, the best solution is to follow Kopia -- set it to true by default and expose a user interface.

yes, I set IgnoreUnknownTypes to True is also refers to the default sets of Kopia.

if we want users to set IgnoreUnknownTypes by themself, what about exposing the interface for users to set all Kopia polices in case of we adding options one by one in future ?

Lyndon-Li · 2023-01-19T10:31:26Z

pkg/uploader/kopia/progress.go

@@ -77,11 +79,17 @@ func (p *KopiaProgress) UploadedBytes(numBytes int64) {
 func (p *KopiaProgress) Error(path string, err error, isIgnored bool) {
 	if isIgnored {
 		atomic.AddInt32(&p.ignoredErrorCount, 1)
+		p.errs = append(p.errs, fmt.Errorf("Ignored error when processing %v: %v", path, err))


Since p.errs is just for log printing in line 131@func (kp *kopiaProvider) RunBackup, why don't we print the log in place here?
If we print the log at the end of the backup, the meaning of the time for each log will be eliminated.
Instead, if we print the log in place, from the time of the log, we know when the error happens, which is even useful for a long running backup.

Meanwhile, p.ignoredErrorCount and p.fatalErrorCount looks a good candidate to print by the end of the backup, e.g., in line 131@func (kp *kopiaProvider) RunBackup

it would be not suitable to output logs in basic public functions and structures, but we could add a timestamp to the error.

In fact, I debug with the Kopia client, and through the error output I found the related Error function .

And our Error in KopiaProgress is imitated from it. through the error message, we could distinct which error is ignored.

it would be not suitable to output logs in basic public functions and structures

Didn't quite get this, but as the built-in handler Error function , it is also printing the log directly in the progress.Error callback.

Lyndon-Li · 2023-01-19T10:32:39Z

pkg/uploader/provider/kopia.go

 	quit := make(chan struct{})
 	log.Info("Starting backup")
 	go kp.CheckContext(ctx, quit, nil, kpUploader)

 	defer func() {
+		snapshotErrs := progress.GetErrors()


See https://github.com/vmware-tanzu/velero/pull/5786/files#r1081073132

Lyndon-Li · 2023-01-19T10:46:06Z

pkg/uploader/kopia/snapshot.go

 	var errs []string
 	if ds := manifest.RootEntry.DirSummary; ds != nil {
 		for _, ent := range ds.FailedEntries {
-			errs = append(errs, ent.Error)
+			if !(*policyTree.DefinedPolicy().ErrorHandlingPolicy.IgnoreUnknownTypes == true && strings.Contains(ent.Error, ErrUnknown)) {


If we really want to ignore any error, we should log something to record that the error has been ignored, otherwise, we will not be able to troubleshoot.

#5786 (comment)

pkg/uploader/kopia/snapshot.go

…ing backup unknown file type Signed-off-by: Ming <mqiu@vmware.com>

reasonerjt

lgtm

XhmikosR · 2023-02-21T06:04:10Z

Doesn't this issue affect the 1.10 branch too? If so, could someone cherry pick the patch there too?

Thanks in advance!

github-actions bot assigned qiuming-best Jan 19, 2023

github-actions bot requested review from blackpiglet and Lyndon-Li January 19, 2023 09:36

qiuming-best force-pushed the unsupport-type-error branch from b634473 to 6550913 Compare January 19, 2023 09:37

github-actions bot added the has-changelog label Jan 19, 2023

qiuming-best requested review from ywk253100 and reasonerjt January 19, 2023 09:39

Lyndon-Li reviewed Jan 19, 2023

View reviewed changes

qiuming-best force-pushed the unsupport-type-error branch 2 times, most recently from b0b43c0 to 2a348ac Compare January 30, 2023 05:59

Lyndon-Li previously approved these changes Jan 30, 2023

View reviewed changes

reasonerjt self-assigned this Feb 6, 2023

reasonerjt reviewed Feb 6, 2023

View reviewed changes

pkg/uploader/kopia/snapshot.go Outdated Show resolved Hide resolved

pkg/uploader/kopia/snapshot.go Outdated Show resolved Hide resolved

qiuming-best dismissed Lyndon-Li’s stale review via 2c13a5f February 7, 2023 07:48

qiuming-best force-pushed the unsupport-type-error branch 2 times, most recently from 2c13a5f to b1a6626 Compare February 7, 2023 08:01

Set Kopia IgnoreUnknownTypes in ErrorHandlingPolicy to True for ignor…

0fd5af3

…ing backup unknown file type Signed-off-by: Ming <mqiu@vmware.com>

qiuming-best force-pushed the unsupport-type-error branch from b1a6626 to 0fd5af3 Compare February 7, 2023 08:31

reasonerjt approved these changes Feb 8, 2023

View reviewed changes

Lyndon-Li approved these changes Feb 13, 2023

View reviewed changes

Lyndon-Li merged commit c5efb54 into vmware-tanzu:main Feb 13, 2023

slawekww mentioned this pull request Feb 20, 2023

Uploader kopia: error for backup not-healthy pods #5882

Closed

sseago mentioned this pull request Aug 31, 2023

sock file fails backup with velero v.1.12 datamover: data path backup failed: Failed to run kopia backup: Error when processing mysql.sock: unknown or unsupported entry type #6736

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set Kopia IgnoreUnknownTypes in ErrorHandlingPolicy to True #5786

Set Kopia IgnoreUnknownTypes in ErrorHandlingPolicy to True #5786

qiuming-best commented Jan 19, 2023 •

edited

Loading

Lyndon-Li Jan 19, 2023 •

edited

Loading

qiuming-best Jan 19, 2023

Lyndon-Li Jan 20, 2023 •

edited

Loading

qiuming-best Jan 20, 2023

Lyndon-Li Jan 19, 2023 •

edited

Loading

qiuming-best Jan 19, 2023

qiuming-best Jan 19, 2023 •

edited

Loading

Lyndon-Li Jan 20, 2023

Lyndon-Li Jan 19, 2023

Lyndon-Li Jan 19, 2023

qiuming-best Jan 19, 2023

reasonerjt left a comment

XhmikosR commented Feb 21, 2023

Set Kopia IgnoreUnknownTypes in ErrorHandlingPolicy to True #5786

Set Kopia IgnoreUnknownTypes in ErrorHandlingPolicy to True #5786

Conversation

qiuming-best commented Jan 19, 2023 • edited Loading

Please add a summary of your change

Does your change fix a particular issue?

Please indicate you've done the following:

Lyndon-Li Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

qiuming-best Jan 19, 2023

Choose a reason for hiding this comment

Lyndon-Li Jan 20, 2023 • edited Loading

Choose a reason for hiding this comment

qiuming-best Jan 20, 2023

Choose a reason for hiding this comment

Lyndon-Li Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

qiuming-best Jan 19, 2023

Choose a reason for hiding this comment

qiuming-best Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

Lyndon-Li Jan 20, 2023

Choose a reason for hiding this comment

Lyndon-Li Jan 19, 2023

Choose a reason for hiding this comment

Lyndon-Li Jan 19, 2023

Choose a reason for hiding this comment

qiuming-best Jan 19, 2023

Choose a reason for hiding this comment

reasonerjt left a comment

Choose a reason for hiding this comment

XhmikosR commented Feb 21, 2023

qiuming-best commented Jan 19, 2023 •

edited

Loading

Lyndon-Li Jan 19, 2023 •

edited

Loading

Lyndon-Li Jan 20, 2023 •

edited

Loading

Lyndon-Li Jan 19, 2023 •

edited

Loading

qiuming-best Jan 19, 2023 •

edited

Loading