CI: Run logformatter for podman_machine_windows_task by l0rd · Pull Request #21991 · containers/podman

l0rd · 2024-03-08T11:05:15Z

That's to address the windows part of #21760

Does this PR introduce a user-facing change?

None

Luap99

Thanks, logs look much better with that

Luap99 · 2024-03-11T17:16:43Z

contrib/cirrus/win-podman-machine-test.ps1

I guess that should be moved to our image builds: https://github.com/containers/automation_images/blob/a3e4099c72052ff21a6e538043cf05b5d873a110/win_images/win_packaging.ps1#L12

Done but untested: containers/automation_images#337 (comment)

Currently we are checking if Perl exists before invoking logformatter and install it if necessary:

podman/contrib/cirrus/win-lib.ps1

Lines 43 to 50 in fb046fc

function Install-Perl-If-Required {

if (-not (Get-Command perl -ErrorAction SilentlyContinue)) {

Write-Host "`nInstalling Perl as it is required to use logformatter"

choco install --no-progress --confirm --acceptlicense --nocolor StrawberryPerl

Write-Host "`nAdd perl to the PATH"

Set-Item "Env:PATH" "$Env:PATH;C:\Strawberry\perl\bin"

}

}

I will open another PR to remove this code as soon as Perl is available in the new windows image.

contrib/cirrus/win-podman-machine-test.ps1

New pasta (03-18). And, probably, lots more changes, some of which will break CI. Also: install StrawberryPerl on Windows, see: containers/podman#21991 (Note: debian crun 1.14.3 still does not exist. Grrr.) First CI-detected problem: debian: The following packages have unmet dependencies: debian: libfuse2t64 : Breaks: libfuse2 (< 2.9.9-8.1) Solution attempted: remove libfuse2 from INSTALL_PACKAGES Signed-off-by: Ed Santiago <santiago@redhat.com>

edsantiago · 2024-03-18T20:33:42Z

Thank you! I'm working on logformatter, adding code to recognize C: and backslashes and that sort of thing.

One request: could you find some way to inject /define.gitCommit=XXXXX into the log output? Without that there's no way to linkify back to source code. Here's how I did it for Macintosh:

podman/Makefile

Lines 632 to 636 in 8a643c2

    
           .PHONY: localmachine 
        
           localmachine: 
        
           	# gitCommit needed by logformatter, to link to sources 
        
           	@echo /define.gitCommit=$(GIT_COMMIT) 
        
           	$(MAKE) ginkgo-run GINKGO_PARALLEL=n TAGS="$(REMOTETAGS)" GINKGO_FLAKE_ATTEMPTS=0 FOCUS_FILE=$(FOCUS_FILE) GINKGOWHAT=pkg/machine/e2e/. HACK=

...but of course that relies on an invocation of the form make localmachine |& logformatter

edsantiago · 2024-03-18T21:31:44Z

@l0rd feel free to grab #22081

New pasta (03-19). And whatever else comes in. Also: install StrawberryPerl on Windows, see: containers/podman#21991 First CI-detected problem: debian: The following packages have unmet dependencies: debian: libfuse2t64 : Breaks: libfuse2 (< 2.9.9-8.1) Solution attempted: remove libfuse2 from INSTALL_PACKAGES Signed-off-by: Ed Santiago <santiago@redhat.com>

New pasta (03-19 and -20). And whatever else comes in. Also: install StrawberryPerl on Windows, see: containers/podman#21991 First CI-detected problem: debian: The following packages have unmet dependencies: debian: libfuse2t64 : Breaks: libfuse2 (< 2.9.9-8.1) Solution attempted: remove libfuse2 from INSTALL_PACKAGES And, bump expired Debian timebombs Signed-off-by: Ed Santiago <santiago@redhat.com>

New pasta (03-20). And whatever else comes in. Also: install StrawberryPerl on Windows, see: containers/podman#21991 First CI-detected problem: debian: The following packages have unmet dependencies: debian: libfuse2t64 : Breaks: libfuse2 (< 2.9.9-8.1) Solution attempted: remove libfuse2 from INSTALL_PACKAGES And, bump expired Debian timebombs Signed-off-by: Ed Santiago <santiago@redhat.com>

l0rd · 2024-03-26T16:11:38Z

I have update the PR so that the html report is generated when tests fail too (this was tricky because there is no way to set pipefail with powershell as we do on bash scripts).

Also now the reports first line has the git commit: /define.gitCommit=<SHA>

Now the windows cirrus tasks install Perl if it's not installed yet. We can merge the PR as is or wait for the new VMs with Perl and remove the code that installs Perl from this PR.

Tests failing examples:

Tests passing examples:

edsantiago · 2024-03-26T16:21:33Z

.cirrus.yml

The logs look great! Thank you! But this spurious /repo breaks all existing conventions, and makes the logs impossible to find. Is there any way to remove the /repo?

Sigh. Doesn't look like there's a way to do that with Cirrus. Can the file be written (or moved) to ..?

The ./repo/ folder is a peculiarity of the windows build (and it took some time to figure that out). I can have a look at the root cause of it. I would like to set ./repo as the working directory to make it consistent with the other platforms. Otherwise I can move the html report up one folder when the log formatter terminates.

Without a 1-click link the formatted logs are not really very useful. However, it's still incremental progress and maybe other Windows peeps will want it.

edsantiago · 2024-03-26T16:24:48Z

Also, you might not know about this: https://github.com/edsantiago/greasemonkey/tree/master/github-ci-highlight

In addition to colorizing github jobs in a way that makes patterns stand out, it also auto-links to colorized logs, adding a single-click trivial way to jump straight to logs. With the /repo thing, this linkification no longer works. Nor does my flake logger.

openshift-ci · 2024-03-26T19:10:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, l0rd

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~~ [edsantiago]

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

containers#21760 Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>

baude · 2024-03-27T13:11:45Z

/lgtm

l0rd · 2024-03-27T13:13:05Z

Also, you might not know about this: edsantiago/greasemonkey@master/github-ci-highlight

In addition to colorizing github jobs in a way that makes patterns stand out, it also auto-links to colorized logs, adding a single-click trivial way to jump straight to logs. With the /repo thing, this linkification no longer works. Nor does my flake logger.

Thanks for sharing. I was not aware of it.

I made an update to the PR to move the report one folder up and now the link works.

edsantiago · 2024-03-27T13:30:39Z

Confirmed, the link now works. Thank you!

Followup to containers#21991. Strawberry Perl is now installed by default in CI VMs[1], so we no longer need the temporary install-perl code. [1] containers/automation_images#337 Signed-off-by: Ed Santiago <santiago@redhat.com>

openshift-ci bot added release-note-none do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 8, 2024

l0rd force-pushed the logformatter-for-win branch 11 times, most recently from c26e30b to 2a6b7a1 Compare March 11, 2024 13:56

l0rd marked this pull request as ready for review March 11, 2024 13:56

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 11, 2024

l0rd mentioned this pull request Mar 11, 2024

CI RFE: logformatter for macos/windows machine tests #21760

Closed

Luap99 reviewed Mar 11, 2024

View reviewed changes

l0rd force-pushed the logformatter-for-win branch from 2a6b7a1 to b8ec448 Compare March 11, 2024 17:55

github-actions bot added the machine label Mar 11, 2024

l0rd marked this pull request as draft March 12, 2024 13:40

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 12, 2024

This was referenced Mar 19, 2024

New VM build, just to see how things are containers/automation_images#337

Merged

logformatter: handle Windows logs #22081

Merged

l0rd force-pushed the logformatter-for-win branch from 4305e60 to 425148a Compare March 26, 2024 13:37

l0rd marked this pull request as ready for review March 26, 2024 16:11

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 26, 2024

edsantiago reviewed Mar 26, 2024

View reviewed changes

edsantiago approved these changes Mar 26, 2024

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2024

l0rd force-pushed the logformatter-for-win branch 3 times, most recently from 810ac29 to 42b45c0 Compare March 26, 2024 23:55

l0rd marked this pull request as draft March 27, 2024 10:20

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 27, 2024

l0rd force-pushed the logformatter-for-win branch from 42b45c0 to de3e1cd Compare March 27, 2024 10:55

Use logformatter for podman_machine_windows_task

c0e32e9

containers#21760 Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>

l0rd force-pushed the logformatter-for-win branch from de3e1cd to c0e32e9 Compare March 27, 2024 11:59

l0rd marked this pull request as ready for review March 27, 2024 13:08

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 27, 2024

openshift-ci bot assigned baude Mar 27, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2024

openshift-merge-bot bot merged commit a77f705 into containers:main Mar 27, 2024

edsantiago mentioned this pull request Apr 3, 2024

Windows: clean up temporary perl install #22248

Merged

stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Jul 3, 2024

stale-locking-app bot locked as resolved and limited conversation to collaborators Jul 3, 2024

	function Install-Perl-If-Required {
	if (-not (Get-Command perl -ErrorAction SilentlyContinue)) {
	Write-Host "`nInstalling Perl as it is required to use logformatter"
	choco install --no-progress --confirm --acceptlicense --nocolor StrawberryPerl
	Write-Host "`nAdd perl to the PATH"
	Set-Item "Env:PATH" "$Env:PATH;C:\Strawberry\perl\bin"
	}
	}

Conversation

l0rd commented Mar 8, 2024

Does this PR introduce a user-facing change?

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

edsantiago commented Mar 18, 2024

Uh oh!

edsantiago commented Mar 18, 2024

Uh oh!

l0rd commented Mar 26, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edsantiago commented Mar 26, 2024

Uh oh!

openshift-ci bot commented Mar 26, 2024

Uh oh!

baude commented Mar 27, 2024

Uh oh!

l0rd commented Mar 27, 2024

Uh oh!

edsantiago commented Mar 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants