Remove unused fields from HorizonTest CR - #474
Conversation
Parameters adminUsername and adminPassword are being deprecated in the test-operator [1]. Their defaults are also being removed, therefore it is needed to remove them in ci-framework as well. Parameters projectName, user, and password were never fully working in the test-operator, as the values were hardcoded regardless of what was set in the CR. The effort to remove them is in test-operator PR [2], which needs their removal in ci-framework as well. [1] openstack-k8s-operators/test-operator#473 [2] openstack-k8s-operators/test-operator#474 Signed-off-by: Katarina Strenkova <kstrenko@redhat.com>
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider FAILURE in 11m 31s |
Parameters adminUsername and adminPassword are being deprecated in the test-operator [1]. Their defaults are also being removed, therefore it is needed to remove them in ci-framework as well. Parameters projectName, user, and password were never fully working in the test-operator, as the values were hardcoded regardless of what was set in the CR. The effort to remove them is in test-operator PR [2], which needs their removal in ci-framework as well. [1] openstack-k8s-operators/test-operator#473 [2] openstack-k8s-operators/test-operator#474 Signed-off-by: Katarina Strenkova <kstrenko@redhat.com>
43ed59d to
2cb266c
Compare
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider FAILURE in 11m 00s |
Parameters adminUsername and adminPassword are being deprecated in the test-operator [1]. Their defaults are also being removed, therefore it is needed to remove them in ci-framework as well. Parameters projectName, user, and password were never fully working in the test-operator, as the values were hardcoded regardless of what was set in the CR. The effort to remove them is in test-operator PR [2], which needs their removal in ci-framework as well. [1] openstack-k8s-operators/test-operator#473 [2] openstack-k8s-operators/test-operator#474 Signed-off-by: Katarina Strenkova <kstrenko@redhat.com>
Parameters adminUsername and adminPassword are being deprecated in the test-operator [1]. Their defaults are also being removed, therefore it is needed to remove them in ci-framework as well. Parameters projectName, user, and password were never fully working in the test-operator, as the values were hardcoded regardless of what was set in the CR. The effort to remove them is in test-operator PR [2], which needs their removal in ci-framework as well. [1] openstack-k8s-operators/test-operator#473 [2] openstack-k8s-operators/test-operator#474 Signed-off-by: Katarina Strenkova <kstrenko@redhat.com>
2cb266c to
ab8152e
Compare
The ProjectName, User, Password fields were never read by the controller. In TCIB it is hardcoded as "horizontest" regardless of what was set in the CR. Same for ImageUrl, FlavorName, LogsDirectory name and HorizonTestDir. Normally, I would recommend deprecation for parameters, but since these parameters never worked from the start, I suggest a removal. There is also no need for them to be parameters, as the Horizon team agreed they do not have to be overwritten and can stay hardcoded.
ab8152e to
68f7908
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 02m 36s |
kstrenkova
added
ready-for-review
|
recheck |
|
For awareness, the |
posikoya
left a comment
posikoya
left a comment
There was a problem hiding this comment.
@kstrenkova this is a great MR and very much needed! :) while going through the initial review everything lgtm. There is only a small thing I noticed, that there may remain the CSV entries in config/manifests/bases/test-operator.clusterserviceversion.yaml for the 7 removed fields (around lines 305-373) for flavorName, horizonTestDir, imageUrl, logsDirectoryName, password, projectName, user
Regenerated with `operator-sdk generate kustomize manifests` to sync specDescriptors with current API type definitions. This command should be run every time a parameter is added/removed, but has been forgotten, hence the update in one go.
Thank you for the review! I forgot that the |
|
@kstrenkova: The following test failed, say
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kstrenkova, posikoya 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 |
2 participants
The ProjectName, User, Password fields were never read by the controller. In TCIB it is hardcoded as "horizontest" regardless of what was set in the CR.
Normally, I would recommend deprecation for parameters, but since these parameters never worked from the start, I suggest a removal. There is also no need for them to be parameters, as the Horizon team agreed they do not have to be overwritten and can stay hardcoded.