USHIFT-1445: fix: prune test

[eggfoobar]

This is to avoid errors in timing between file creation and when the system comes up, since the prune logic skips touching the directory if backup exists, this tests could fail sometimes if ran out of order.

Moving to one test to avoid issue.

/assign @pmtk

[openshift-ci-robot]

@eggfoobar: This pull request references USHIFT-1445 which is a valid jira issue.

Details

In response to this:

This is to avoid errors in timing between file creation and when the system comes up, since the prune logic skips touching the directory if backup exists, this tests could fail sometimes if ran out of order.

Moving to one test to avoid issue.

/assign @pmtk

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[eggfoobar]

@dhellmann and @pmtk I updated the test but I also ran across why things were failing, the issue was that we prune all directories with an underscore, is that desired or should we post fix something to our backups?

[dhellmann]

@dhellmann and @pmtk I updated the test but I also ran across why things were failing, the issue was that we prune all directories with an underscore, is that desired or should we post fix something to our backups?

Is there any reason for us to tolerate anything in that directory that we don't create?

[dhellmann]

/lgtm

Let's get CI working and come back to the question about how much we should retain or how aggressively we should delete things.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann, eggfoobar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [dhellmann,eggfoobar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[eggfoobar]

@dhellmann and @pmtk I updated the test but I also ran across why things were failing, the issue was that we prune all directories with an underscore, is that desired or should we post fix something to our backups?

Is there any reason for us to tolerate anything in that directory that we don't create?

Currently, we place a failed log there that we push up to greenboot, which does have the name prerun_failed.log, that won't be deleted before it's presented to greenboot, since it's a result of a backup failure and subsequent failed restarts mean that that file won't be removed, but just something to keep in mind for other future instances where we store files there.

[dhellmann]

@dhellmann and @pmtk I updated the test but I also ran across why things were failing, the issue was that we prune all directories with an underscore, is that desired or should we post fix something to our backups?

Is there any reason for us to tolerate anything in that directory that we don't create?

Currently, we place a failed log there that we push up to greenboot, which does have the name prerun_failed.log, that won't be deleted before it's presented to greenboot, since it's a result of a backup failure and subsequent failed restarts mean that that file won't be removed, but just something to keep in mind for other future instances where we store files there.

We put that in the backup directory? Could it live in /usr/lib/microshift instead?

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD c3a8c05 and 2 for PR HEAD 957dc76 in total

[pmtk]

@dhellmann and @pmtk I updated the test but I also ran across why things were failing, the issue was that we prune all directories with an underscore, is that desired or should we post fix something to our backups?

Seems like I've skipped few thought steps lgtming previous PR 😆
This is actually intentional, how do you differentiate between known and unknown? We take all backups matching %s_%s and remove them if deployment id is no longer on the system.

Currently, we place a failed log there that we push up to greenboot, which does have the name prerun_failed.log, that won't be deleted before it's presented to greenboot, since it's a result of a backup failure and subsequent failed restarts mean that that file won't be removed, but just something to keep in mind for other future instances where we store files there.

Does prerun_failed.log gets deleted when pruning the backups? dataManager's GetBackupList() should only return directories.
I wouldn't rely on "backup pruning" to remove that file (even sounds like a side effect, if that works that is, but it shouldn't)

[eggfoobar]

Does prerun_failed.log gets deleted when pruning the backups? dataManager's GetBackupList() should only return directories. I wouldn't rely on "backup pruning" to remove that file (even sounds like a side effect, if that works that is, but it shouldn't)

Nah, by default on startup that file gets deleted and only get's written when a failure happens, so pruning never gets a chance to touch it. On failures, greenboot will read that file during healthchecks

[openshift-ci]

@eggfoobar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/microshift-e2e-arm	957dc76	link	false	/test microshift-e2e-arm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.