Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MGMT-15736: Align feature support level to support HighAvailabilityMode as filterable feature #5473

Conversation

eliorerz
Copy link
Contributor

@eliorerz eliorerz commented Sep 10, 2023

Add parameter to api/assisted-install/v2/support-levels/features API so that the features could be filtered also by high_availability_mod, e.g.

api/assisted-install/v2/support-levels/features?openshift_version=4.10&cpu_architecture=x86_64?high_availability_mode=Full

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

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

/cc @eliorerz

@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 10, 2023

@eliorerz: This pull request references MGMT-15736 which is a valid jira issue.

In response to this:

Add parameter to api/assisted-install/v2/support-levels/features API so that the features could be filtered also by high_availability_mod, e.g.

api/assisted-install/v2/support-levels/features?openshift_version=4.10&cpu_architecture=x86_64?high_availability_mod=Full

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

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

/cc @eliorerz

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.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 10, 2023
@openshift-ci 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 Sep 10, 2023
@openshift-ci
Copy link

openshift-ci bot commented Sep 10, 2023

@eliorerz: GitHub didn't allow me to request PR reviews from the following users: eliorerz.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Add parameter to api/assisted-install/v2/support-levels/features API so that the features could be filtered also by high_availability_mod, e.g.

api/assisted-install/v2/support-levels/features?openshift_version=4.10&cpu_architecture=x86_64?high_availability_mod=Full

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

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

/cc @eliorerz

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.

@openshift-ci
Copy link

openshift-ci bot commented Sep 10, 2023

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

@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. api-review Categorizes an issue or PR as actively needing an API review. labels Sep 10, 2023
@openshift-ci
Copy link

openshift-ci bot commented Sep 10, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eliorerz

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2023
@eliorerz eliorerz force-pushed the MGMT-15736_Align_feature_support_level_to_support_HighAvailabilityMode_as_filterable_feature branch from 2598dd0 to 3b5159d Compare September 10, 2023 09:41
@eliorerz
Copy link
Contributor Author

/test edge-unit-test edge-subsystem-aws edge-subsystem-kubeapi-aws

@codecov
Copy link

codecov bot commented Sep 10, 2023

Codecov Report

Attention: Patch coverage is 82.08955% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 68.46%. Comparing base (4c82b47) to head (db440e8).
Report is 289 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5473      +/-   ##
==========================================
+ Coverage   67.63%   68.46%   +0.82%     
==========================================
  Files         232      232              
  Lines       33988    35397    +1409     
==========================================
+ Hits        22989    24235    +1246     
- Misses       8951     9067     +116     
- Partials     2048     2095      +47     
Files Coverage Δ
internal/featuresupport/common.go 87.23% <100.00%> (+0.87%) ⬆️
internal/featuresupport/features_misc.go 87.90% <100.00%> (-1.27%) ⬇️
internal/provider/nutanix/installConfig.go 0.00% <0.00%> (ø)
internal/provider/vsphere/installConfig.go 0.00% <0.00%> (ø)
internal/cluster/validations/validations.go 24.07% <0.00%> (+0.35%) ⬆️
...ernal/featuresupport/architecture_support_level.go 85.71% <60.00%> (-14.29%) ⬇️
internal/featuresupport/feature_support_level.go 90.00% <95.00%> (+0.71%) ⬆️
internal/featuresupport/features_olm_operators.go 78.94% <71.42%> (+0.32%) ⬆️
internal/featuresupport/features_platforms.go 93.08% <91.66%> (+1.05%) ⬆️
internal/featuresupport/features_networking.go 81.39% <66.66%> (-1.94%) ⬇️
... and 1 more

... and 10 files with indirect coverage changes

@eliorerz eliorerz marked this pull request as ready for review September 11, 2023 10:06
@openshift-ci 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 Sep 11, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 11, 2023

@eliorerz: This pull request references MGMT-15736 which is a valid jira issue.

In response to this:

Add parameter to api/assisted-install/v2/support-levels/features API so that the features could be filtered also by high_availability_mod, e.g.

api/assisted-install/v2/support-levels/features?openshift_version=4.10&cpu_architecture=x86_64?high_availability_mode=Full

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

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

/cc @eliorerz

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.

@eliorerz
Copy link
Contributor Author

/cc @avishayt @gamli75

@eliorerz eliorerz force-pushed the MGMT-15736_Align_feature_support_level_to_support_HighAvailabilityMode_as_filterable_feature branch 2 times, most recently from b291553 to 8d1ddf7 Compare September 11, 2023 11:53
@@ -22,6 +22,10 @@ func (feature *SnoFeature) GetName() string {
}

func (feature *SnoFeature) getSupportLevel(filters SupportLevelFilters) models.SupportLevel {
if filters.HighAvailabilityMode != nil {
return ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you are doing this, can you explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, when returning empty string as support level it is not included in the response

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I'll try to implement something more clear

Expect(len(list)).To(Equal(16))
}
})
It("GetFeatureSupportList 4.12 with HighAvailabilityMode", func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
It("GetFeatureSupportList 4.12 with HighAvailabilityMode", func() {
It("GetFeatureSupportList 4.12 with HighAvailabilityModeNone", func() {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test check the features count if HighAvailabilityMode is set, this test is not HighAvailabilityModeNone specific

OpenshiftVersion: "4.12",
CPUArchitecture: nil,
PlatformType: nil,
HighAvailabilityMode: swag.String(models.ClusterHighAvailabilityModeNone),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have another test with ClusterHighAvailabilityModeFull that should return a different length number of the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If HighAvailabilityMode is set, it doesn't matter what is the value

@eliorerz
Copy link
Contributor Author

/retest

@eliorerz eliorerz force-pushed the MGMT-15736_Align_feature_support_level_to_support_HighAvailabilityMode_as_filterable_feature branch from 8d1ddf7 to 0f0dfcb Compare September 13, 2023 06:24
@eliorerz eliorerz force-pushed the MGMT-15736_Align_feature_support_level_to_support_HighAvailabilityMode_as_filterable_feature branch from 0f0dfcb to db440e8 Compare September 13, 2023 11:22
@openshift-ci openshift-ci bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 13, 2023
@eliorerz
Copy link
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 13, 2023
@eliorerz
Copy link
Contributor Author

/test ?

@openshift-ci
Copy link

openshift-ci bot commented Sep 13, 2023

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

  • /test e2e-agent-compact-ipv4
  • /test edge-assisted-operator-catalog-publish-verify
  • /test edge-ci-index
  • /test edge-e2e-ai-operator-ztp
  • /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-9
  • /test edge-e2e-metal-assisted-cnv
  • /test edge-e2e-metal-assisted-lvm
  • /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-ha-dualstack
  • /test e2e-agent-sno-ipv6
  • /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-hypershift-zero-nodes
  • /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-node-labels
  • /test edge-e2e-ai-operator-ztp-sno-day2-masters
  • /test edge-e2e-ai-operator-ztp-sno-day2-workers-ignitionoverride
  • /test edge-e2e-metal-assisted-4-13
  • /test edge-e2e-metal-assisted-4-14
  • /test edge-e2e-metal-assisted-cnv-4.14
  • /test edge-e2e-metal-assisted-day2
  • /test edge-e2e-metal-assisted-day2-arm-workers
  • /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-mce
  • /test edge-e2e-metal-assisted-mce-4-10
  • /test edge-e2e-metal-assisted-mce-sno
  • /test edge-e2e-metal-assisted-metallb
  • /test edge-e2e-metal-assisted-none
  • /test edge-e2e-metal-assisted-odf-4-14
  • /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-metal-assisted-upgrade-agent
  • /test edge-e2e-nutanix-assisted
  • /test edge-e2e-nutanix-assisted-2workers
  • /test edge-e2e-oci-assisted
  • /test edge-e2e-vsphere-assisted
  • /test edge-e2e-vsphere-assisted-4-12
  • /test edge-e2e-vsphere-assisted-4-13
  • /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-e2e-agent-compact-ipv4
  • 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-nutanix-assisted
  • pull-ci-openshift-assisted-service-master-edge-e2e-oci-assisted
  • pull-ci-openshift-assisted-service-master-edge-e2e-vsphere-assisted
  • 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.

@eliorerz
Copy link
Contributor Author

/test edge-e2e-oci-assisted edge-e2e-metal-assisted-none

@eliorerz
Copy link
Contributor Author

/test edge-e2e-nutanix-assisted

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2023
@openshift-merge-robot
Copy link

PR needs rebase.

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.

Copy link

openshift-ci bot commented Nov 14, 2023

@eliorerz: 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/edge-e2e-metal-assisted db440e8 link true /test edge-e2e-metal-assisted
ci/prow/edge-e2e-oci-assisted db440e8 link false /test edge-e2e-oci-assisted
ci/prow/edge-e2e-vsphere-assisted db440e8 link false /test edge-e2e-vsphere-assisted
ci/prow/edge-e2e-metal-assisted-none db440e8 link false /test edge-e2e-metal-assisted-none
ci/prow/edge-e2e-ai-operator-ztp db440e8 link true /test edge-e2e-ai-operator-ztp
ci/prow/edge-e2e-nutanix-assisted db440e8 link false /test edge-e2e-nutanix-assisted
ci/prow/edge-e2e-oci-assisted-4-14 db440e8 link true /test edge-e2e-oci-assisted-4-14

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.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 13, 2024
@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 14, 2024
@eifrach
Copy link
Contributor

eifrach commented Mar 14, 2024

duplicate #6077
/close

@openshift-ci openshift-ci bot closed this Mar 14, 2024
Copy link

openshift-ci bot commented Mar 14, 2024

@eifrach: Closed this PR.

In response to this:

duplicate #6077
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants