Fix: Include platform info in the manifest for iHealth #217
Conversation
There was a problem hiding this comment.
Pull Request Overview
Enhances the iHealth integration by adding platform and product information to the manifest. The changes collect platform-specific metadata (platform type, build, serial number, hostname) and product details from NGINX Ingress Controller pods, storing this information in JSON files that are then included in the generated manifest.
- Adds collection of platform information (OS, architecture, hostnames) from Kubernetes nodes
- Implements parsing of NGINX Ingress Controller version and build information
- Updates manifest generation to include collected platform and product data
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/jobs/nic_job_list.go | Adds product info collection job and parsing function for NGINX Ingress Controller version data |
| pkg/jobs/common_job_list.go | Extends node collection to extract platform information including OS details and hostnames |
| pkg/data_collector/data_collector.go | Updates data structures and manifest generation to include platform and product information |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Potential panic if hostnames slice is empty. Should check len(hostnames) > 0 before accessing hostnames[0]. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Empty error handling with only a comment. Should either handle the error appropriately or remove the empty block. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Using log.Fatalf will terminate the entire program if product_info.json doesn't exist. Should handle the error gracefully and continue with default values instead of crashing. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
The jsonResult variable contains marshaled data from the result variable, but then it's being unmarshaled back into a NodeList. This is unnecessary - the original result should be used directly since it's already a NodeList type. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Remove superfluous log message introduced by co-pilot review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Check the existence of the `control-plane` label to confirm that it is the control node. Co-authored-by: Daniel Aresté <5310624+dareste@users.noreply.github.com>
Minor camel case fix Co-authored-by: Daniel Aresté <5310624+dareste@users.noreply.github.com>
…b of product-platform-info
2 participants
Proposed changes
As part of the iHealth integration, the manifest now includes these additional information:
platform_typebuildserial_numberhostnameChecklist
Before creating a PR, run through this checklist and mark each as complete.