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

Adds Progressing status to BackupBucketsReady seed condition #6587

Merged
merged 7 commits into from
Sep 5, 2022
Merged

Adds Progressing status to BackupBucketsReady seed condition #6587

merged 7 commits into from
Sep 5, 2022

Conversation

plkokanov
Copy link
Contributor

How to categorize this PR?

/area ops-productivity
/kind enhancement

What this PR does / why we need it:
Adds a Progressing status to the BackupBucketsReady seed condition. When a BackupBucket that refers to a seed becomes erroneous, the BackupBucketsReady condition will first be set to Progressing, if the configured condition threshold has not expired. Once the threshold expires, the condition will be set to False

Which issue(s) this PR fixes:
Part of #4722

Special notes for your reviewer:

Release note:

The `BackupBucketsReady` condition for `Seed`s will first be set to `Progressing` instead of being directly set to `False` when a `BackupBucketsReady` condition threshold is specified in the `controllers.seedBackupBucketsCheck.conditionThresholds` configuration for the gardener controller manager and that threshold has not expired yet.

@gardener-prow gardener-prow bot added area/ops-productivity Operator productivity related (how to improve operations) kind/enhancement Enhancement, improvement, extension labels Aug 26, 2022
@gardener-prow gardener-prow bot added cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 26, 2022
@plkokanov plkokanov changed the title Adds Progressing status to BackupBucketsReady seed condition Adds Progressing status to BackupBucketsReady seed condition Aug 26, 2022
@plkokanov plkokanov mentioned this pull request Aug 26, 2022
Closed
@shafeeqes shafeeqes self-assigned this Aug 29, 2022
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 29, 2022
shafeeqes
shafeeqes reviewed Aug 30, 2022
View reviewed changes
Copy link
Contributor

@shafeeqes shafeeqes left a comment

Choose a reason for hiding this comment

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

Thank you!
I have only few minor comments.

pkg/controllermanager/controller/seed/seed_backup_bucket_check_control.go Outdated Show resolved Hide resolved
pkg/controllermanager/controller/seed/seed_backup_bucket_check_control_test.go Show resolved Hide resolved
ialidzhikov
ialidzhikov reviewed Aug 31, 2022
View reviewed changes
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

lgtm in general

pkg/controllermanager/controller/seed/seed_backup_bucket_check_control.go Outdated Show resolved Hide resolved
pkg/controllermanager/controller/seed/seed_backup_bucket_check_control.go Show resolved Hide resolved
@plkokanov plkokanov force-pushed the enh/progressing-backup-bucket-condition branch from 1bcf997 to c276880 Compare September 2, 2022 13:29
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2022
@rfranzke
Copy link
Member

rfranzke commented Sep 2, 2022

/rebase

@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2022
@rfranzke rfranzke self-assigned this Sep 2, 2022
@plkokanov plkokanov force-pushed the enh/progressing-backup-bucket-condition branch from c276880 to 7820a24 Compare September 2, 2022 17:21
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2022
@shafeeqes
Copy link
Contributor

@plkokanov: 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
pull-gardener-apidiff 7820a24 link false /test pull-gardener-apidiff
pull-gardener-e2e-kind 7820a24 link true /test pull-gardener-e2e-kind
Full PR test history. Your PR dashboard. Command help for this repository. Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

Opened #6623
/retest

@gardener-prow
Copy link
Contributor

gardener-prow bot commented Sep 5, 2022 •
edited

@plkokanov: 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-gardener-apidiff 7820a24 link false /test pull-gardener-apidiff

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

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.

@rfranzke
Copy link
Member

rfranzke commented Sep 5, 2022

/lgtm
/approve

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2022
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Sep 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

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

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2022
@gardener-prow gardener-prow bot merged commit b6cb420 into gardener:master Sep 5, 2022
krgostev pushed a commit to krgostev/gardener that referenced this pull request Sep 8, 2022
Merge branch 'master' of github.com:gardener/gardener into master
fe66a40 
* 'master' of github.com:gardener/gardener: (121 commits)
  Added metrics needed for multi-node etcd and backup-restore. (gardener#6601)
  Fix beta failing test for arm (gardener#6632)
  Install yq in e2e CI jobs (gardener#6631)
  Upgrade etcd-druid (gardener#6630)
  Switch `SecretBinding` controller to `controller-runtime` (gardener#6621)
  Fix the network metrics for clusters with containerd (gardener#6628)
  Remove release milestone github action (gardener#6627)
  Adds `Progressing` status to `BackupBucketsReady` seed condition  (gardener#6587)
  Adds more info when comparing secrets for cpm integration tests (gardener#6622)
  Update profiling docs (gardener#6624)
  Add `Progressing` condition type to `ControllerInstallations` and check it when computing the `ExtensionsReady` condition (gardener#6590)
  Deny setting `.spec.kubernetes.allowPrivilegedContainers` field for Shoot clusters `>=v1.25` (gardener#6570)
  Use v3.24.1 manifests for calico (gardener#6616)
  Exempt `kube-system` namespace in `PodSecurity` admission config (gardener#6549)
  Make sure the `default-scheduler` is present even when bin-packing is enabled (gardener#6610)
  Upgrade etcd-druid (gardener#6611)
  Adapt blackbox exporter resource requests to VPA recommendations (gardener#6609)
  Clean up unused search textbox in the `Kubernetes Control Plane Status` dashboard (gardener#6606)
  Reduce the size of the grafana dashboards configmap (gardener#6605)
  Fix bug in resourcemanager that ignore annotation does handle all possible truthy values (gardener#6603)
  ...
@plkokanov plkokanov deleted the enh/progressing-backup-bucket-condition branch December 9, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ialidzhikov ialidzhikov ialidzhikov left review comments

@rfranzke rfranzke rfranzke left review comments

@shafeeqes shafeeqes shafeeqes left review comments

@ary1992 ary1992 Awaiting requested review from ary1992

Assignees

@rfranzke rfranzke

@shafeeqes shafeeqes

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/ops-productivity Operator productivity related (how to improve operations) cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@plkokanov @rfranzke @shafeeqes @ialidzhikov