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

Allow editing and disabling common DataImportCronTemplates #1832

Merged
merged 1 commit into from
Mar 27, 2022

Conversation

nunnatsa
Copy link
Collaborator

@nunnatsa nunnatsa commented Mar 24, 2022

HCO now supports the removing of a specific DataImportCronTemplate from the list in SSP. Also, HCO allow modying the storage field.

HyperConverged API Changes:

New field: status.dataImportCronTemplates

This field is a list of the actual dataImportCronTemplates as HCO updated the SSP CR. The list contains both the common and the user-defined templates (the list in the spec field contains only the user-defined templates).

The purpose of this field is for transparency, so the user could see the actual templates. It should also ease the modification of the templates, because it will give the information about the common-templates, which are currently not visible to the user.

The templates in the status.dataImportCronTemplates list contains a new status object with two fields:

  • commonTemplate; boolean; indicates whether this is a common template (true), or a custom one (false).
  • modified; boolean; indicates if a common template was customized. Always false for custom templates.

For example:

apiVersion: hco.kubevirt.io/v1beta1
kind: HyperConverged
metadata:
  name: kubevirt-hyperconverged
spec:
  
status:
  dataImportCronTemplates:
  - metadata:
      name: rhel8-image-cron
    spec:
      
    status:
      commonTemplate: true
      Modified: false
  - metadata:
      name: my-castom-image
    spec:
      
    status:
      commonTemplate: false

New annotation: dataimportcrontemplate.kubevirt.io/enable

spec.dataImportCronTemplates[*].metadata.annotations["dataimportcrontemplate.kubevirt.io/enable"]
Boolean string. Default = "true"

This new annotation in the dataImportCronTemplate object, if set to false, signals HCO not to set this template in the SSP CR, or remove the template from the SSP CR if it is currently listed there. For common templates, the specific template is recognized by its name. For example, in order to disable the common "rhel8-image-cron" template, add this to the spec:

apiVersion: hco.kubevirt.io/v1beta1
kind: HyperConverged
metadata:
  name: kubevirt-hyperconverged
spec:
  dataImportCronTemplates:
  - metadata:
      annotations:
        dataimportcrontemplate.kubevirt.io/enable: false
      name: rhel8-image-cron

HCO Changes

Currently, the HCO webhook rejects changes to the HyperConverged CR, if the spec.dataImportCronTemplates list contains a common template. After this change, HCO will not reject such requests. Instead, HCO allows to modify the following fields in the common templates:

  • spec.dataImportCronTemplates[*].spec.template.spec.storage

For example

apiVersion: hco.kubevirt.io/v1beta1
kind: HyperConverged
metadata:
  name: kubevirt-hyperconverged
spec:
  dataImportCronTemplates:
  - metadata:
      name: rhel8-image-cron
    spec:
      template:
        spec:
          storageClassName: "some-name"

If this field is added to the HyperConverged spec, HCO will replace the storage field if exist, or add it if it is missing.

Any other field will be ignored.

  • If the DataImportCronTemplate’s dataimportcrontemplate.kubevirt.io/enable annotation is set to false, HCO will not set this template in the SSP CR, or delete it from SSP CR if it is there.

HCO will copy the template list, as written to the SSP CR, to the new status.dataImportCronTemplates field, with addition of two optional fields: commonTemplate and modified, as described above. If the template is set to “disabled” in the spec, it won’t appear in the status.

Signed-off-by: Nahshon Unna-Tsameret nunnatsa@redhat.com

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.

  • PR Message
  • Commit Messages
  • How to test
  • Unit Tests
  • Functional Tests
  • User Documentation
  • Developer Documentation
  • Upgrade Scenario
  • Uninstallation Scenario
  • Backward Compatibility
  • Troubleshooting Friendly

Release note:

HCO now supports the removing of a specific DataImportCronTemplate from the list in SSP. Also, HCO allow modying the storage field.

@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 24, 2022
@nunnatsa nunnatsa requested a review from orenc1 March 24, 2022 14:33
@coveralls
Copy link
Collaborator

coveralls commented Mar 24, 2022

Pull Request Test Coverage Report for Build 2046982878

  • 101 of 101 (100.0%) changed or added relevant lines in 12 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 84.772%

Files with Coverage Reduction New Missed Lines %
controllers/operands/operandHandler.go 1 87.58%
Totals Coverage Status
Change from base Build 2038902143: 0.2%
Covered Lines: 3869
Relevant Lines: 4564

💛 - Coveralls

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2022
@tiraboschi
Copy link
Member

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tiraboschi

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 24, 2022
@hco-bot
Copy link
Collaborator

hco-bot commented Mar 25, 2022

hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/hco-e2e-image-index-aws

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-aws

In response to this:

hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/hco-e2e-image-index-aws

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.

@nunnatsa
Copy link
Collaborator Author

/retest
/test pull-hyperconverged-cluster-operator-e2e-k8s-1.21

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 26, 2022
@hco-bot
Copy link
Collaborator

hco-bot commented Mar 26, 2022

okd-hco-e2e-upgrade-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-upgrade-index-aws
okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-sno-aws, ci/prow/okd-hco-e2e-image-index-aws, ci/prow/okd-hco-e2e-upgrade-index-aws

In response to this:

okd-hco-e2e-upgrade-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-upgrade-index-aws
okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

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.

@nunnatsa
Copy link
Collaborator Author

/retest

@nunnatsa
Copy link
Collaborator Author

/override-bot

@hco-bot
Copy link
Collaborator

hco-bot commented Mar 26, 2022

hco-e2e-upgrade-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-aws

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-index-aws

In response to this:

hco-e2e-upgrade-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-aws

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.

@nunnatsa
Copy link
Collaborator Author

/retest

HCO now supports the removing of a specific DataImportCronTemplate from the list in SSP. Also, HCO allow modying the storage field.

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2022
@sonarcloud
Copy link

sonarcloud bot commented Mar 27, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@openshift-ci
Copy link

openshift-ci bot commented Mar 27, 2022

@nunnatsa: 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-kv-smoke-azure 25bf656 link true /test hco-e2e-kv-smoke-azure
ci/prow/hco-e2e-image-index-azure 25bf656 link true /test hco-e2e-image-index-azure
ci/prow/hco-e2e-upgrade-prev-index-azure 25bf656 link true /test hco-e2e-upgrade-prev-index-azure

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.

@hco-bot
Copy link
Collaborator

hco-bot commented Mar 27, 2022

hco-e2e-upgrade-prev-index-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-azure
hco-e2e-image-index-gcp, hco-e2e-image-index-aws lanes succeeded.
/override ci/prow/hco-e2e-image-index-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-azure, ci/prow/hco-e2e-upgrade-prev-index-azure

In response to this:

hco-e2e-upgrade-prev-index-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-azure
hco-e2e-image-index-gcp, hco-e2e-image-index-aws lanes succeeded.
/override ci/prow/hco-e2e-image-index-azure

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.

@hco-bot
Copy link
Collaborator

hco-bot commented Mar 27, 2022

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure

In response to this:

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

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.

@nunnatsa
Copy link
Collaborator Author

/test pull-hyperconverged-cluster-operator-e2e-k8s-1.21

@openshift-ci
Copy link

openshift-ci bot commented Mar 27, 2022

@nunnatsa: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test ci-index
  • /test ci-index-hco-upgrade-bundle
  • /test ci-index-hco-upgrade-prev-bundle
  • /test hco-e2e-image-index-aws
  • /test hco-e2e-image-index-azure
  • /test hco-e2e-image-index-gcp
  • /test hco-e2e-kv-smoke-azure
  • /test hco-e2e-kv-smoke-gcp
  • /test hco-e2e-upgrade-index-aws
  • /test hco-e2e-upgrade-index-azure
  • /test hco-e2e-upgrade-prev-index-aws
  • /test hco-e2e-upgrade-prev-index-azure
  • /test images
  • /test okd-ci-index
  • /test okd-ci-index-hco-upgrade-bundle
  • /test okd-ci-index-hco-upgrade-prev-bundle
  • /test okd-hco-e2e-image-index-aws
  • /test okd-hco-e2e-image-index-gcp
  • /test okd-hco-e2e-upgrade-index-aws
  • /test okd-hco-e2e-upgrade-index-gcp
  • /test okd-images

The following commands are available to trigger optional jobs:

  • /test hco-e2e-image-index-sno-aws
  • /test hco-e2e-image-index-sno-azure
  • /test hco-e2e-upgrade-index-sno-aws
  • /test hco-e2e-upgrade-index-sno-azure
  • /test hco-e2e-upgrade-prev-index-sno-aws
  • /test hco-e2e-upgrade-prev-index-sno-azure

Use /test all to run all jobs.

In response to this:

/test pull-hyperconverged-cluster-operator-e2e-k8s-1.21

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.

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2022
@kubevirt-bot kubevirt-bot merged commit 61e4c93 into kubevirt:main Mar 27, 2022
@nunnatsa nunnatsa deleted the modify-dic-templates branch March 28, 2022 05:48
orenc1 added a commit to orenc1/hyperconverged-cluster-operator that referenced this pull request Apr 1, 2022
Signed-off-by: orenc1 <ocohen@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Apr 3, 2022
* Kubevirt Plugin Integration

Add Operand Handlers to create and reconcile new resources for the Kubvirt console plugin in OpenShift:
1. Deployment for the kubevirt plugin, serving the UI contents using nginx
2. Service that exposes the kubevirt plugin pod
3. Custom Resource for the plugin - ConsolePlugin

Signed-off-by: orenc1 <ocohen@redhat.com>

* Adding unit tests and enable kubevirt-plugin in consoles.operator.openshift.io

Signed-off-by: orenc1 <ocohen@redhat.com>

* add new hook method from #1832 to the new handlers

Signed-off-by: orenc1 <ocohen@redhat.com>

* fixes according to review comments

Signed-off-by: orenc1 <ocohen@redhat.com>

* not creating console plugin operands if console plugin image is not configured

Signed-off-by: orenc1 <ocohen@redhat.com>

* configure the plugin deployment and service with SSL

Signed-off-by: orenc1 <ocohen@redhat.com>

* pin kubevirt-plugin image to a specific version and disable the feature until a release is available

Signed-off-by: orenc1 <ocohen@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants