Consolidate e2e log collection

[jsturtevant]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
We have several different entry points for our CI and some of them use different logging scripts. This creates a binary that re-uses to the same logging as our conformance and e2e. It takes some basic configuration and runs the same logging functions used by the e2e_testsuite. An added benefit is that a user could also run this and collect logs that are the same as the e2e test.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1372

Special notes for your reviewer:

This required moving around some of the capz functions in the e2e test pacakge similiar to what was done in #2130

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

• squashed commits
• includes documentation
• adds unit tests

Release note:

Consolidate e2e logging methods

[Jont828]

@jsturtevant Is this ready for review or is it still WIP? Saw that there was a broken link in the tests.

[jsturtevant]

The other tests are passing, The folder structure isn't 100% the same so going to look into that but the general approach is ready for review.

The broken link is to a file that is in this PR, so not sure how we handle that. It doesn't exist but when this gets published it would exist...

[Jont828]

You can use <!-- markdown-link-check-disable-line --> to have it ignore the broken link for now. We use it for when we reference http://localhost:<port> as well.

[jsturtevant]

You can use <!-- markdown-link-check-disable-line --> to have it ignore the broken link for now. We use it for when we reference http://localhost:<port> as well.

Then we follow up with a separate PR to remove it? I feel that it would be better to merge knowing the check is only failing a link in that will be there once this merges but let me know what we typically do for this scenario.

[CecileRobertMichon]

/retitle Consolidate e2e log collection

[jsturtevant]

The folder structure for ci-entrypoint.sh is now the same as the ci-e2e.sh and ci-conformance.sh:

artifacts/
└── clusters
    ├── bootstrap
    │   ├── controllers
    │   │   ├── capi-controller-manager
    │   │   ├── capi-kubeadm-bootstrap-controller-manager
    │   │   ├── capi-kubeadm-control-plane-controller-manager
    │   │   └── capz-controller-manager
    │   └── resources
    │       └── default
    │           ├── AzureCluster
    │           ├── AzureClusterIdentity
    │           ├── AzureMachine
    │           ├── AzureMachineTemplate
    │           ├── Cluster
    │           ├── KubeadmConfig
    │           ├── KubeadmConfigTemplate
    │           ├── KubeadmControlPlane
    │           ├── Machine
    │           ├── MachineDeployment
    │           ├── MachineHealthCheck
    │           └── MachineSet
    └── capz-conf-r00mm3
        └── azure-activity-logs
        └── kube-system
        └── machines
            ├── capz-conf-r00mm3-md-win-8675ffb97b-8z89g 
            └── capz-conf-r00mm3-md-win-8675ffb97b-xmmhl

[jsturtevant]

fyi @lzhecheng @nilo19 @andyzhangx

[CecileRobertMichon]

The broken link is to a file that is in this PR, so not sure how we handle that. It doesn't exist but when this gets published it would exist...

this is a chicken-and-egg problem... no matter what it will require a second PR if we want to job to pass on this PR. We can also ignore the linter and merge the PR with the failure but that's not ideal.

[jsturtevant]

this is a chicken-and-egg problem... no matter what it will require a second PR if we want to job to pass on this PR. We can also ignore the linter and merge the PR with the failure but that's not ideal.

Which approach is preferred?

[jsturtevant]

/test pull-cluster-api-provider-azure-e2e

[Jont828]

An alternative approach would be to open a stub PR including the just the file we are linking in the docs (assuming the file isn't going to produce compile errors without the rest being present). Then, you could rebase this PR off of that one. Otherwise, I would personally lean towards using the disable link check for now.

[CecileRobertMichon]

do we really need to take the RG as input here or can we figure that out from the azure cluster spec?

[jsturtevant]

I hadn't considered using the cluster spec, I will see if that can simplify adding an additional flag

[jsturtevant]

I fixed it at the place in the code where we are using an env variable instead of looking it up on the cluster spec

[jsturtevant]

https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/2345/files#diff-34d5acd96ae3a8b3040a0d8a711797ef5eaceabe05de690d663fa6cb670cead5R221-R227

[CecileRobertMichon]

/lgtm

[CecileRobertMichon]

/assign @mboersma

[mboersma]

/lgtm

This is a great improvement! Thanks for addressing my ticky-tack comments.

[CecileRobertMichon]

/approve
/cherry-pick release-1.3

[k8s-infra-cherrypick-robot]

@CecileRobertMichon: once the present PR merges, I will cherry-pick it on top of release-1.3 in a new PR and assign it to you.

In response to this:

/approve
/cherry-pick release-1.3

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.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [CecileRobertMichon]

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

[CecileRobertMichon]

--- FAIL: TestAzureMachinePool_Validate (0.00s)
    --- FAIL: TestAzureMachinePool_Validate/HasValidImage (0.00s)
        azuremachinepool_test.go:66: 
            Unexpected error:
                <*field.Error | 0xc00031c040>: {
                    Type: "FieldValueForbidden",
                    Field: "spec",
                    BadValue: <string>"",
                    Detail: "can be set only if the MachinePool feature flag is enabled",
                }
                spec: Forbidden: can be set only if the MachinePool feature flag is enabled
            occurred

This failure seems to be coming from #2376 which is strange because it passed all tests before merging

[jsturtevant]

is it happening consistently and for other prs?

/test pull-cluster-api-provider-azure-test

[CecileRobertMichon]

hmm retest passed 🤔

I have not seen it on other PRs, but let's keep an 👁️ out. If it's non-deterministic that's an issue.

[k8s-infra-cherrypick-robot]

@CecileRobertMichon: #2345 failed to apply on top of branch "release-1.3":

Applying: refactor logging so it is same in all scripts
.git/rebase-apply/patch:28: trailing whitespace.
which you can also leverage to pull all the logs for machines which will dump logs to `${PWD}/_artifacts}` by default. The following works 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	scripts/ci-entrypoint.sh
M	test/e2e/e2e_suite_test.go
Falling back to patching base and 3-way merge...
Auto-merging test/e2e/e2e_suite_test.go
CONFLICT (content): Merge conflict in test/e2e/e2e_suite_test.go
Auto-merging scripts/ci-entrypoint.sh
CONFLICT (content): Merge conflict in scripts/ci-entrypoint.sh
Removing hack/log/log-dump.sh
Removing hack/log/log-dump-daemonset.yaml
Removing hack/log/log-dump-daemonset-windows.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 refactor logging so it is same in all scripts
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/approve
/cherry-pick release-1.3

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.