MGMT-12312: Adds node-boot log type #4529

nmagnezi · 2022-10-20T15:53:52Z

Per #4444, add a new log type named: node-boot.

This change updates both V2UploadLogs and V2DownloadClusterLogs to accept the new log type.
In addition:

A new event was added: host_boot_logs_uploaded.
Defined download name: <cluster_name>_<host_role>_boot_<host_id>.tar.gz.
Note that V2UploadLogs with log type node-boot will not update the host progress.

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
No tests needed

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

openshift-ci · 2022-10-20T15:53:59Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

nmagnezi · 2022-10-20T15:54:09Z

this is WIP

openshift-ci · 2022-10-20T15:55:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nmagnezi

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

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

nmagnezi · 2022-10-23T13:00:44Z

/test ?

openshift-ci · 2022-10-23T13:00:52Z

@nmagnezi: The following commands are available to trigger required jobs:

/test edge-assisted-operator-catalog-publish-verify
/test edge-ci-index
/test edge-e2e-ai-operator-ztp
/test edge-e2e-ai-operator-ztp-converged
/test edge-e2e-ai-operator-ztp-sno-day2-workers
/test edge-e2e-ai-operator-ztp-sno-day2-workers-late-binding
/test edge-e2e-metal-assisted
/test edge-e2e-metal-assisted-4-10
/test edge-e2e-metal-assisted-4-11
/test edge-e2e-metal-assisted-4-12
/test edge-e2e-metal-assisted-4-8
/test edge-e2e-metal-assisted-4-9
/test edge-e2e-metal-assisted-cnv
/test edge-e2e-metal-assisted-lvm
/test edge-e2e-metal-assisted-ocs
/test edge-e2e-metal-assisted-odf
/test edge-images
/test edge-lint
/test edge-subsystem-aws
/test edge-subsystem-kubeapi-aws
/test edge-unit-test
/test edge-verify-generated-code
/test images
/test mce-images

The following commands are available to trigger optional jobs:

/test e2e-agent-compact
/test edge-e2e-ai-operator-ztp-3masters
/test edge-e2e-ai-operator-ztp-capi
/test edge-e2e-ai-operator-ztp-compact-day2-masters
/test edge-e2e-ai-operator-ztp-compact-day2-workers
/test edge-e2e-ai-operator-ztp-disconnected
/test edge-e2e-ai-operator-ztp-ipv4v6-3masters
/test edge-e2e-ai-operator-ztp-ipv4v6-3masters-ocp-411
/test edge-e2e-ai-operator-ztp-ipv4v6-sno
/test edge-e2e-ai-operator-ztp-ipv4v6-sno-ocp-411
/test edge-e2e-ai-operator-ztp-multiarch-3masters-ocp-411
/test edge-e2e-ai-operator-ztp-multiarch-sno-ocp-411
/test edge-e2e-ai-operator-ztp-sno-day2-masters
/test edge-e2e-ai-operator-ztp-sno-day2-workers-ignitionoverride
/test edge-e2e-metal-assisted-capi
/test edge-e2e-metal-assisted-day2
/test edge-e2e-metal-assisted-day2-single-node
/test edge-e2e-metal-assisted-ipv4v6
/test edge-e2e-metal-assisted-ipv6
/test edge-e2e-metal-assisted-kube-api-late-binding-single-node
/test edge-e2e-metal-assisted-kube-api-late-unbinding-ipv4-single-node
/test edge-e2e-metal-assisted-kube-api-net-suite
/test edge-e2e-metal-assisted-none
/test edge-e2e-metal-assisted-onprem
/test edge-e2e-metal-assisted-single-node
/test edge-e2e-metal-assisted-static-ip-suite
/test edge-e2e-metal-assisted-tang
/test edge-e2e-metal-assisted-tpmv2
/test edge-e2e-vsphere-assisted
/test edge-e2e-vsphere-assisted-umn
/test edge-push-pr-image
/test push-pr-image

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-assisted-service-master-edge-ci-index
pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp
pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted
pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted-capi
pull-ci-openshift-assisted-service-master-edge-images
pull-ci-openshift-assisted-service-master-edge-lint
pull-ci-openshift-assisted-service-master-edge-subsystem-aws
pull-ci-openshift-assisted-service-master-edge-subsystem-kubeapi-aws
pull-ci-openshift-assisted-service-master-edge-unit-test
pull-ci-openshift-assisted-service-master-edge-verify-generated-code
pull-ci-openshift-assisted-service-master-images
pull-ci-openshift-assisted-service-master-mce-images

In response to this:

/test ?

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.

nmagnezi · 2022-10-23T13:01:18Z

/test edge-lint
/test edge-unit-test

codecov · 2022-10-23T13:31:43Z

Codecov Report

Merging #4529 (77d0d73) into master (b4879ac) will increase coverage by 2.03%.
The diff coverage is 93.33%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4529      +/-   ##
==========================================
+ Coverage   67.78%   69.82%   +2.03%     
==========================================
  Files         202      202              
  Lines       30093    32179    +2086     
==========================================
+ Hits        20398    22468    +2070     
+ Misses       7888     7855      -33     
- Partials     1807     1856      +49

Impacted Files	Coverage Δ
internal/bminventory/inventory_v2_handlers.go	`61.08% <81.81%> (+0.19%)`	⬆️
internal/bminventory/inventory.go	`79.14% <100.00%> (+9.30%)`	⬆️
internal/cluster/validations/validations.go	`26.07% <0.00%> (+0.26%)`	⬆️
internal/oc/release.go	`71.83% <0.00%> (+0.81%)`	⬆️
internal/network/utils.go	`37.54% <0.00%> (+1.49%)`	⬆️
...nternal/controller/controllers/agent_controller.go	`80.32% <0.00%> (+1.64%)`	⬆️
internal/host/hostcommands/install_cmd.go	`84.16% <0.00%> (+1.69%)`	⬆️
...oller/controllers/clusterdeployments_controller.go	`78.81% <0.00%> (+4.52%)`	⬆️

nmagnezi · 2022-10-24T06:12:05Z

/assign @avishayt
/assign @tsorya
/assign @filanov

nmagnezi · 2022-10-24T06:12:18Z

/assign @carbonin

nmagnezi · 2022-10-24T13:14:10Z

/retest-required

nmagnezi · 2022-10-30T20:14:17Z

/hold

nmagnezi · 2022-12-08T09:52:40Z

/test edge-e2e-ai-operator-ztp

openshift-ci · 2022-12-08T11:07:28Z

@nmagnezi: 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/edge-e2e-metal-assisted-capi	`0e4cc03`	link	false	`/test edge-e2e-metal-assisted-capi`

Full PR test history. Your PR dashboard.

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.

nmagnezi · 2022-12-08T13:55:09Z

/unhold
ci/prow/edge-e2e-ai-operator-ztp fail because the db pod did not start:

ERROR:  role "admin" does not exist

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_assisted-service/4529/pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp/1600790493363441664/artifacts/e2e-ai-operator-ztp/gather-extra/artifacts/pods/assisted-installer_assisted-service-6bc8577999-t49bp_postgres.log

Other PRs did succeed, so trying again.

nmagnezi · 2022-12-08T13:55:22Z

/test edge-e2e-ai-operator-ztp

carbonin · 2022-12-08T16:22:19Z

Looks like there's still quite a few questions in the enhancement.
The code looks good though. I'm curious about "boot logs" as a name though since we boot the host at least twice.

Maybe we need a more specific name since we're putting it in a user-facing event.
Maybe just saying node boot rather than host would be better?
Something like post install boot logs might work but is a bit confusing because we're done installing coreos, but the node isn't quite done yet.

@avishayt what do you think?

avishayt · 2022-12-28T06:36:12Z

Agree with @carbonin . node-boot-logs is better than host-boot-logs, maybe boot-from-disk-logs is more explicit?

Per assisted-service/pull/4444[1] , add a new log type named: node-boot. This change updates both V2UploadLogs and V2DownloadClusterLogs to accept the new log type. In addition: 1. A new event was added: host_boot_logs_uploaded. 2. Defined download name: <cluster_name>_<host_role>_boot_<host_id>.tar.gz 3. Note that V2UploadLogs with log type node-boot will not update the host progress. [1] openshift#4444

nmagnezi · 2023-01-08T13:33:30Z

Agree with @carbonin . node-boot-logs is better than host-boot-logs, maybe boot-from-disk-logs is more explicit?

@carbonin, @avishayt,
I changed the log name to node-boot

avishayt · 2023-01-08T16:01:38Z

Excellent, thanks

carbonin · 2023-01-11T21:20:38Z

/lgtm

Per assisted-service/pull/4444[1] , add a new log type named: node-boot. This change updates both V2UploadLogs and V2DownloadClusterLogs to accept the new log type. In addition: 1. A new event was added: host_boot_logs_uploaded. 2. Defined download name: <cluster_name>_<host_role>_boot_<host_id>.tar.gz 3. Note that V2UploadLogs with log type node-boot will not update the host progress. [1] openshift#4444

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 Oct 20, 2022

openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 20, 2022

openshift-ci bot added the api-review Categorizes an issue or PR as actively needing an API review. label Oct 20, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 20, 2022

nmagnezi force-pushed the MGMT-12312_new_log_type branch from c0c4689 to 173962c Compare October 23, 2022 10:50

openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 23, 2022

nmagnezi force-pushed the MGMT-12312_new_log_type branch from 173962c to e47e8ca Compare October 23, 2022 13:00

nmagnezi changed the title ~~Mgmt 12312 new log type~~ MGMT-12312: Adds host-boot log type Oct 23, 2022

nmagnezi marked this pull request as ready for review October 23, 2022 13:31

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 Oct 23, 2022

openshift-ci bot requested review from javipolo and ori-amizur October 23, 2022 13:31

nmagnezi force-pushed the MGMT-12312_new_log_type branch from e47e8ca to 0e4cc03 Compare October 24, 2022 06:07

openshift-ci bot assigned avishayt, filanov and tsorya Oct 24, 2022

openshift-ci bot assigned carbonin Oct 24, 2022

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 30, 2022

nmagnezi force-pushed the MGMT-12312_new_log_type branch from 0e4cc03 to 3b81197 Compare December 8, 2022 06:29

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 8, 2022

nmagnezi force-pushed the MGMT-12312_new_log_type branch from 3b81197 to 1c63288 Compare January 8, 2023 13:30

nmagnezi force-pushed the MGMT-12312_new_log_type branch from 1c63288 to 77d0d73 Compare January 8, 2023 13:32

nmagnezi changed the title ~~MGMT-12312: Adds host-boot log type~~ MGMT-12312: Adds node-boot log type Jan 9, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 11, 2023

openshift-merge-robot merged commit 49f633a into openshift:master Jan 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-12312: Adds node-boot log type #4529

MGMT-12312: Adds node-boot log type #4529

nmagnezi commented Oct 20, 2022 •

edited

Loading

openshift-ci bot commented Oct 20, 2022

nmagnezi commented Oct 20, 2022

openshift-ci bot commented Oct 20, 2022

nmagnezi commented Oct 23, 2022

openshift-ci bot commented Oct 23, 2022

nmagnezi commented Oct 23, 2022

codecov bot commented Oct 23, 2022 •

edited

Loading

nmagnezi commented Oct 24, 2022

nmagnezi commented Oct 24, 2022

nmagnezi commented Oct 24, 2022

nmagnezi commented Oct 30, 2022

nmagnezi commented Dec 8, 2022

openshift-ci bot commented Dec 8, 2022 •

edited

Loading

nmagnezi commented Dec 8, 2022

nmagnezi commented Dec 8, 2022

carbonin commented Dec 8, 2022

avishayt commented Dec 28, 2022

nmagnezi commented Jan 8, 2023

avishayt commented Jan 8, 2023

carbonin commented Jan 11, 2023

MGMT-12312: Adds node-boot log type #4529

MGMT-12312: Adds node-boot log type #4529

Conversation

nmagnezi commented Oct 20, 2022 • edited Loading

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci bot commented Oct 20, 2022

nmagnezi commented Oct 20, 2022

openshift-ci bot commented Oct 20, 2022

nmagnezi commented Oct 23, 2022

openshift-ci bot commented Oct 23, 2022

nmagnezi commented Oct 23, 2022

codecov bot commented Oct 23, 2022 • edited Loading

Codecov Report

nmagnezi commented Oct 24, 2022

nmagnezi commented Oct 24, 2022

nmagnezi commented Oct 24, 2022

nmagnezi commented Oct 30, 2022

nmagnezi commented Dec 8, 2022

openshift-ci bot commented Dec 8, 2022 • edited Loading

nmagnezi commented Dec 8, 2022

nmagnezi commented Dec 8, 2022

carbonin commented Dec 8, 2022

avishayt commented Dec 28, 2022

nmagnezi commented Jan 8, 2023

avishayt commented Jan 8, 2023

carbonin commented Jan 11, 2023

nmagnezi commented Oct 20, 2022 •

edited

Loading

codecov bot commented Oct 23, 2022 •

edited

Loading

openshift-ci bot commented Dec 8, 2022 •

edited

Loading