kubevirt / hyperconverged-cluster-operator

Operator pattern for managing multi-operator products
Apache License 2.0
153 stars 152 forks source link

Make `applicationAwareConfig` fields optional #3019

Closed orenc1 closed 2 months ago

orenc1 commented 3 months ago

In order for the UI not to render a default value for applicationAwareConfigsubfields for the HyperConverged custom resource, those should be set as optional as well.

What this PR does / why we need it:

Reviewer Checklist

Reviewers are supposed to review the PR for every aspect below one by one. To check an item means the PR is either "OK" or "Not Applicable" in terms of that item. All items are supposed to be checked before merging a PR.

Jira Ticket:

https://issues.redhat.com/browse/CNV-40208

Release note:

Make applicationAwareConfig fields optional
kubevirt-bot commented 3 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from orenc1. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubevirt/hyperconverged-cluster-operator/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9860158755

Details


Totals Coverage Status
Change from base Build 9842265518: 0.02%
Covered Lines: 5212
Relevant Lines: 6068

💛 - Coveralls
nunnatsa commented 2 months ago

@orenc1 I don't think this change will help. The problem is not in HCO, I think. As you can see, the CRD was not changed by your change, and the CRD is the input to the UI.

Also, it was never marked as as a required field in the CRD, so it's already optional.

orenc1 commented 2 months ago

@orenc1 I don't think this change will help. The problem is not in HCO, I think. As you can see, the CRD was not changed by your change, and the CRD is the input to the UI.

Also, it was never marked as as a required field in the CRD, so it's already optional.

but permittedHostDevices and its subfields are also marked as optional, and they are not appearing by default in the UI on HyperConverged creation page. how it is possible?

nunnatsa commented 2 months ago

permittedHostDevices

I can guess it's because these fields are array (slices), and this one is boolean.

Anyway, this is not an HCO bug.

orenc1 commented 2 months ago

permittedHostDevices

I can guess it's because these fields are array (slices), and this one is boolean.

Anyway, this is not an HCO bug.

I think it's because we're setting default values. I'm checking what happens when the default is not being set (PR is updated)

/hold

kubevirt-bot commented 2 months ago

@orenc1: 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
pull-hyperconverged-cluster-operator-e2e-k8s-1.29 1567756902199cba87729cbc3679aed899a4dfd5 link true /test pull-hyperconverged-cluster-operator-e2e-k8s-1.29
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
openshift-ci[bot] commented 2 months ago

@orenc1: The following tests 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/hco-e2e-upgrade-prev-operator-sdk-aws 1567756902199cba87729cbc3679aed899a4dfd5 link true /test hco-e2e-upgrade-prev-operator-sdk-aws
ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure 1567756902199cba87729cbc3679aed899a4dfd5 link true /test hco-e2e-consecutive-operator-sdk-upgrades-azure
ci/prow/hco-e2e-kv-smoke-azure 1567756902199cba87729cbc3679aed899a4dfd5 link true /test hco-e2e-kv-smoke-azure

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
nunnatsa commented 2 months ago

See #2788