NO-JIRA: Fix Infrastructure PlatformType comment #2660
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@2uasimojo: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
📝 WalkthroughWalkthroughPlatform enumeration documentation and schemas were updated to add "IBMCloud" and "External" as valid platform values. Changes touch PlatformSpec/type comments, generated Swagger/OpenAPI documentation, and multiple CRD OpenAPI schemas and payload manifests. No Go type signatures, function signatures, or runtime control flow were modified. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
|
Hello @2uasimojo! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/XS
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
|
/assign @JoelSpeed So:
|
IIUC we don't use enums because it can break backward compatibility. |
2uasimojo
force-pushed
the
infra-platformtype-comment
branch
from
19c87dc to
ef70680
Compare
openshift-ci
bot
added
size/L
|
/test verify |
|
PR-Agent: could not fine a component named |
|
Looks like we need a |
| // "OpenStack", "VSphere", "oVirt", "KubeVirt", "EquinixMetal", "PowerVS", | ||
| // "AlibabaCloud", "Nutanix" and "None". Individual components may not support all platforms, | ||
| // and must handle unrecognized platforms as None if they do not support that platform. | ||
| // "OpenStack", "VSphere", "oVirt", "IBMCloud", "KubeVirt", "EquinixMetal", |
There was a problem hiding this comment.
While this now reflects the list, we don't actually support Equinix or Alibaba, and I thought oVirt went too. We should probably think about removing those constants from the API at some point
There was a problem hiding this comment.
...but not in this PR?
There was a problem hiding this comment.
IIUC the ways this list can be used, we wouldn't want to remove a given value until the last OCP release that supports it is EOL, right?
There was a problem hiding this comment.
Quite possibly, of which, it would be the oVirt part. I think that is EOL now though , it went in ~4.14 I think
The comment for the config/v1/Infrastructure.Spec.PlatformSpec.Type was outdated with respect to the consts defined for the PlatformType typedef. Update.
2uasimojo
force-pushed
the
infra-platformtype-comment
branch
from
ef70680 to
e87c917
Compare
✓ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-Default.crd.yaml (1)
2482-2513: UpdatePlatformStatus.typedescription to list all supported platform values.The description in the source file (config/v1/types_infrastructure.go, line 418-425) is missing "IBMCloud", "KubeVirt", and "External" from the allowed values list, while the enum correctly includes them. Update the description to match PlatformSpec.type, which lists all 16 supported platforms consistently.
payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yaml (1)
2904-2907: UpdatePlatformStatus.Typedescription to include all supported platforms.The source Go type comment for
PlatformStatus.Typeinconfig/v1/types_infrastructure.go(lines 422-423) is missing "IBMCloud", "KubeVirt", and "External" from its description, whilePlatformSpec.Type(lines 305-306) has been updated to include them. The enum validation already supports all platforms. Update thePlatformStatus.Typecomment to match, then regenerate the CRD by running./hack/update-payload-crds.sh.payload-manifests/crds/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yaml (1)
2898-2929: Update thestatus.platformStatus.typedescription to include all supported platform types.The description at lines 2904-2905 lists allowed values as "AWS", "Azure", "BareMetal", "GCP", "Libvirt", "OpenStack", "VSphere", "oVirt", "EquinixMetal", "PowerVS", "AlibabaCloud", "Nutanix" and "None", but the enum at lines 2911-2928 also includes
IBMCloud,KubeVirt, andExternal. The source code inconfig/v1/types_infrastructure.godocuments all these platforms. The description should be updated to match the enum; regenerating the CRD manifests from the source types should resolve this.
🤖 Fix all issues with AI agents
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml`:
- Around line 631-634: The description for status.platformStatus.type is out of
date—update the text to match the spec type enumeration (including "IBMCloud",
"External", and any other newly added platforms such as "OpenStack", "VSphere",
"oVirt", "KubeVirt", "EquinixMetal", "PowerVS", "AlibabaCloud", "Nutanix", and
"None"); locate the description for status.platformStatus.type (and the OKD
variant) in the CRD and replace the old platform list with the exact updated
list used by spec.type so both descriptions are consistent.
🧹 Nitpick comments (2)
payload-manifests/crds/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml (1)
2597-2627: Consider updatingstatus.platformStatus.typedescription for consistency.The
status.platformStatus.typedescription (lines 2603-2606) lists platforms but is missing "IBMCloud", "KubeVirt", and "External" despite these values being present in its enum. This is outside the scope of the current PR focused onspec.platformSpec.type, but may warrant a follow-up for consistency.config/v1/zz_generated.swagger_doc_generated.go (1)
1926-1928: AlignPlatformStatus.typeallowed values with the updatedPlatformSpeclist.
PlatformSpec.typenow documents IBMCloud/External (and KubeVirt), butPlatformStatus.typestill lists an older subset. For Swagger doc consistency, consider updating the status description in the source comment and regenerating the file.♻️ Suggested doc alignment (update source + regenerate)
- "type": "type is the underlying infrastructure provider for the cluster. This value controls whether infrastructure automation such as service load balancers, dynamic volume provisioning, machine creation and deletion, and other integrations are enabled. If None, no infrastructure automation is enabled. Allowed values are \"AWS\", \"Azure\", \"BareMetal\", \"GCP\", \"Libvirt\", \"OpenStack\", \"VSphere\", \"oVirt\", \"EquinixMetal\", \"PowerVS\", \"AlibabaCloud\", \"Nutanix\" and \"None\". Individual components may not support all platforms, and must handle unrecognized platforms as None if they do not support that platform.\n\nThis value will be synced with to the `status.platform` and `status.platformStatus.type`. Currently this value cannot be changed once set.", + "type": "type is the underlying infrastructure provider for the cluster. This value controls whether infrastructure automation such as service load balancers, dynamic volume provisioning, machine creation and deletion, and other integrations are enabled. If None, no infrastructure automation is enabled. Allowed values are \"AWS\", \"Azure\", \"BareMetal\", \"GCP\", \"Libvirt\", \"OpenStack\", \"VSphere\", \"oVirt\", \"IBMCloud\", \"KubeVirt\", \"EquinixMetal\", \"PowerVS\", \"AlibabaCloud\", \"Nutanix\", \"External\", and \"None\". Individual components may not support all platforms, and must handle unrecognized platforms as None if they do not support that platform.\n\nThis value will be synced with to the `status.platform` and `status.platformStatus.type`. Currently this value cannot be changed once set.",
| "OpenStack", "VSphere", "oVirt", "IBMCloud", "KubeVirt", "EquinixMetal", | ||
| "PowerVS", "AlibabaCloud", "Nutanix", "External", and "None". Individual | ||
| components may not support all platforms, and must handle unrecognized | ||
| platforms as None if they do not support that platform. |
There was a problem hiding this comment.
Keep status.platformStatus.type description aligned with this updated list.
The spec type description now includes IBMCloud/External, but the later status.platformStatus.type description in this CRD still lists the older platform set. Please update that description (and the OKD variant) so the docs don’t conflict.
🤖 Prompt for AI Agents
In
`@payload-manifests/crds/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml`
around lines 631 - 634, The description for status.platformStatus.type is out of
date—update the text to match the spec type enumeration (including "IBMCloud",
"External", and any other newly added platforms such as "OpenStack", "VSphere",
"oVirt", "KubeVirt", "EquinixMetal", "PowerVS", "AlibabaCloud", "Nutanix", and
"None"); locate the description for status.platformStatus.type (and the OKD
variant) in the CRD and replace the old platform list with the exact updated
list used by spec.type so both descriptions are consistent.
|
/lgtm No need for verification, docs only update |
openshift-ci-robot
added
the
verified
|
@JoelSpeed: The DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@2uasimojo: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
3 participants
User description
The comment for the config/v1/Infrastructure.Spec.PlatformSpec.Type was outdated with respect to the consts defined for the PlatformType typedef. Update.
PR Type
Documentation
Description
Update PlatformType comment to reflect current platform constants
Add missing "IBMCloud" and "External" platform types
Improve comment formatting and readability
Diagram Walkthrough
File Walkthrough
types_infrastructure.go
Update PlatformType comment with missing platformsconfig/v1/types_infrastructure.go
"External" platforms