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.

Sign up for GitHub

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

Jump to bottom

Add enhancement for suspending and resuming scan schedules #375

Merged
merged 1 commit into from Oct 12, 2023
Merged

Add enhancement for suspending and resuming scan schedules #375

merged 1 commit into from Oct 12, 2023

Conversation

rhmdnd
Copy link

@rhmdnd rhmdnd commented Aug 1, 2023

This commit includes an enhancement for implementing a feature that
pauses and resumes existing ComplianceScans.

@openshift-ci openshift-ci bot requested review from jhrozek and xiaojiey August 1, 2023 21:48
@openshift-ci openshift-ci bot added the approved label Aug 1, 2023
rhmdnd
rhmdnd commented Aug 1, 2023
View reviewed changes
enhancements/pause-and-resume-scans.md Outdated
stable feature of Kubernetes.

### Operational Aspects of API Extensions

Copy link
Author

Choose a reason for hiding this comment

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

This functionality will add an additional API call to deal with CronJobs during the ComplianceScan reconcile loops, which we already have to deal with as we setup the scan rerunners.

mkumku
mkumku reviewed Aug 2, 2023
View reviewed changes
enhancements/pause-and-resume-scans.md Outdated
- Minor version upgrades - you only need to support `x.N->x.N+1` upgrade
steps. So, for example, it is acceptable to require a user running 4.3 to
upgrade to 4.5 with a `4.3->4.4` step followed by a `4.4->4.5` step.
- While an upgrade is in progress, new component versions should
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this enough. I am concern though if the paused scan will still hold reliable results after the upgrade of a cluster. Or, on another hand, if there is an upgrade in the scan profile - how do we apply this retroactively to a paused scan. To me it still feels maybe better to delete a scan and start a new after maintenance then resume an old scan.

Copy link
Author

Choose a reason for hiding this comment

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

I attempted to describe this is in more detail in the next revision of the patch.

Here we're specifically pausing the scan schedule, and not stopping a scan while it is in progress. Does that help clarify?

Vincent056
Vincent056 requested changes Aug 28, 2023
View reviewed changes
Copy link

@Vincent056 Vincent056 left a comment

Choose a reason for hiding this comment

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

I think the proposal looks good overall, just have some concerns on where annotation should be applied.

enhancements/suspend-and-resume-scan-schedule.md Outdated Show resolved Hide resolved
@rhmdnd rhmdnd changed the title Add enhancement for pausing and resuming scans Add enhancement for suspending and resuming scan schedules Aug 29, 2023
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Aug 31, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
a4fd0f0 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
@rhmdnd rhmdnd mentioned this pull request Aug 31, 2023
Merged
@rhmdnd
Copy link
Author

rhmdnd commented Aug 31, 2023

/retest

@rhmdnd
Copy link
Author

rhmdnd commented Aug 31, 2023

I can update to rebase to exclude e2e tests for this patch, since they're not relevant and consume resources.

openshift/release#42834

@openshift-ci
Copy link

openshift-ci bot commented Aug 31, 2023

@rhmdnd: 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
ci/prow/e2e-aws-serial 29306d6 link true /test e2e-aws-serial

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.

rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Aug 31, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
064cdef 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Aug 31, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
d41b4e9 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Aug 31, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
e8f84a3 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
@rhmdnd
Copy link
Author

rhmdnd commented Sep 1, 2023

@Vincent056 let me know if the latest revision addresses your concerns.

rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Sep 1, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
25f04d3 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
@xiaojiey
Copy link
Collaborator

/hold for test

rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Sep 21, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
0f8c102 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Sep 22, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
132c6ac 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Sep 27, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
d1b79f9 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Sep 28, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
1c1ee63 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
@rhmdnd
Copy link
Author

rhmdnd commented Sep 29, 2023

I think the proposal looks good overall, just have some concerns on where annotation should be applied.

I think I updated all references for that in the enhancement and implementation. Let me know if you see any other issues.

rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 5, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
37e9636 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 5, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
5f117d8 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 5, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
8170852 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 5, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
34bc4e6 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 5, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
2306474 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 6, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
7c27fe9 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 6, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
b4fedde 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
rhmdnd added a commit to rhmdnd/compliance-operator that referenced this pull request Oct 6, 2023
@rhmdnd
CMP-2132: Implement suspend and resume scan schedule
919e395 
This commit implements the logic and tests necessary to suspend and
resume scan schedules using the `ScanSetting` custom resource.

You can find more details on the overall justification, use cases, and
implementation details in the enhancement:

  ComplianceAsCode#375
@xiaojiey
Copy link
Collaborator

xiaojiey commented Oct 9, 2023

/unhold
/label qe-approved

@rhmdnd
Add enhancement for suspending and resuming scan schedules
d75ca49 
This commit includes an enhancement for implementing a feature that
suspends and resumes existing ComplianceScans schedules.
yuumasato
yuumasato approved these changes Oct 12, 2023
View reviewed changes
Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

/lgtm

Thank you for the well written reasoning and implementation details.

@openshift-ci openshift-ci bot added the lgtm label Oct 12, 2023
@openshift-ci
Copy link

openshift-ci bot commented Oct 12, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhmdnd, yuumasato

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 merged commit bfdf204 into ComplianceAsCode:master Oct 12, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuumasato yuumasato yuumasato approved these changes

@jhrozek jhrozek Awaiting requested review from jhrozek

@xiaojiey xiaojiey Awaiting requested review from xiaojiey

@sheriff-rh sheriff-rh Awaiting requested review from sheriff-rh

@Vincent056 Vincent056 Awaiting requested review from Vincent056

@mkumku mkumku Awaiting requested review from mkumku

Assignees

@yuumasato yuumasato

@Vincent056 Vincent056

Labels
approved docs-approved lgtm qe-approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@rhmdnd @xiaojiey @yuumasato @mkumku @Vincent056 @sheriff-rh