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

OCPBUGS-17113: rework unstructured types for monitoring #1575

Merged
merged 11 commits into from
Aug 29, 2024
Merged

OCPBUGS-17113: rework unstructured types for monitoring #1575

merged 11 commits into from
Aug 29, 2024

Conversation

rexagod
Copy link
Member

@rexagod rexagod commented Sep 21, 2023

This PR incorporates the following changes:

  • refactoring the existing functions (and comments) a bit,
  • cache-based apply for unstructured monitoring types, and,
  • adding explicit *KnownUnstructured functions for alertmanager and prometheus.

Slack discussion thread for the same.

/cc @dgrisonnet

@openshift-ci openshift-ci bot requested a review from dgrisonnet September 21, 2023 09:49
@rexagod rexagod changed the title resourceapply: rework unstructured types for monitoring OCPBUGS-17113: rework unstructured types for monitoring Sep 21, 2023
@openshift-ci-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Sep 21, 2023
@openshift-ci-robot
Copy link

@rexagod: This pull request references Jira Issue OCPBUGS-17113, which is invalid:

  • expected the bug to target the "4.15.0" version, but it targets "4.14.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

This PR incorporates the following changes:

  • refactoring the existing functions (and comments) a bit,
  • cache-based apply for unstructured monitoring types, and,
  • adding explicit *KnownUnstructured functions for alertmanager and prometheus.

Slack discussion thread for the same.

/cc @dgrisonnet

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/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Sep 21, 2023
@rexagod rexagod mentioned this pull request Sep 21, 2023
Closed
@rexagod
Copy link
Member Author

rexagod commented Sep 21, 2023

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 21, 2023
@openshift-ci-robot
Copy link

@rexagod: This pull request references Jira Issue OCPBUGS-17113, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.15.0) matches configured target version for branch (4.15.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @juzhao

In response to this:

/jira refresh

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 openshift-ci bot requested a review from juzhao September 21, 2023 09:57
@rexagod
Copy link
Member Author

rexagod commented Oct 31, 2023

/retest

@rexagod
Copy link
Member Author

rexagod commented Oct 31, 2023 •
edited
Loading

  • Additionally, look into utilizing observedGeneration and resourceVersion (already done AFAICT) fields whenever present to better cache based on object spec and metadata changes.

@jan--f jan--f mentioned this pull request Dec 5, 2023
Closed
@dgrisonnet
Copy link
Member

I completely missed that PR, @rexagod is this still relevant?

@rexagod
Copy link
Member Author

rexagod commented Dec 6, 2023

@dgrisonnet ACK! We we'd be happy to see this go in, albeit it seems I missed the unit test failure here.

@rexagod
Copy link
Member Author

rexagod commented Dec 11, 2023

/retest

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2024
@rexagod rexagod force-pushed the monitoring-cache-based-unstructured branch from 85ebb7f to 0a6ed40 Compare February 20, 2024 11:52
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2024
@rexagod
Copy link
Member Author

rexagod commented Feb 20, 2024

/test unit

@rexagod
Copy link
Member Author

rexagod commented Feb 27, 2024

(friendly ping @dgrisonnet)

@rexagod
Copy link
Member Author

rexagod commented Mar 13, 2024

cc @deads2k

@rexagod rexagod force-pushed the monitoring-cache-based-unstructured branch from 87498e1 to e6edd22 Compare May 3, 2024 15:45
@rexagod rexagod requested a review from deads2k May 7, 2024 07:30
@rexagod
Copy link
Member Author

rexagod commented May 22, 2024

(bump)

1 similar comment
@rexagod
Copy link
Member Author

rexagod commented May 28, 2024

(bump)

@deads2k
Copy link
Contributor

deads2k commented Aug 29, 2024

/approve

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Aug 29, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, dusk125, jan--f, rexagod

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 Aug 29, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Aug 29, 2024

@rexagod: all tests passed!

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-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit aaabd80 into openshift:master Aug 29, 2024
4 checks passed
@openshift-ci-robot
Copy link

@rexagod: Jira Issue OCPBUGS-17113: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-17113 has not been moved to the MODIFIED state.

In response to this:

This PR incorporates the following changes:

  • refactoring the existing functions (and comments) a bit,
  • cache-based apply for unstructured monitoring types, and,
  • adding explicit *KnownUnstructured functions for alertmanager and prometheus.

Slack discussion thread for the same.

/cc @dgrisonnet

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 openshift-eng/jira-lifecycle-plugin repository.

rexagod added a commit to rexagod/cluster-monitoring-operator that referenced this pull request Sep 9, 2024
@rexagod
OCPBUGS-17113: Lazy updates for Prometheus
0c417da 
Incorporate openshift/library-go#1575
downstream.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/cluster-monitoring-operator that referenced this pull request Sep 9, 2024
@rexagod
OCPBUGS-17113: Lazy updates for Prometheus
15b56ea 
Incorporate openshift/library-go#1575
downstream.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/cluster-monitoring-operator that referenced this pull request Sep 19, 2024
@rexagod
OCPBUGS-17113: Lazy updates for Prometheus
5665e69 
Incorporate openshift/library-go#1575
downstream.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/cluster-monitoring-operator that referenced this pull request Oct 7, 2024
@rexagod
OCPBUGS-17113: Lazy updates for Prometheus
0d7194a 
Incorporate openshift/library-go#1575
downstream.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
@p0lyn0mial p0lyn0mial mentioned this pull request Oct 8, 2024
Merged
@p0lyn0mial
Copy link
Contributor

Unfortunately this PR doesn't work as expected, see #1575 (comment) for more info.

rexagod added a commit to rexagod/library-go that referenced this pull request Oct 8, 2024
@rexagod
regression: Set resource version for CRs
56405d4 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 8, 2024
@rexagod
regression: Set resource version for CRs
56b50e8 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
@rexagod rexagod mentioned this pull request Oct 8, 2024
Merged
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 11, 2024
@rexagod
regression: Set resource version for CRs
1231e02 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 11, 2024
@rexagod
Set resource version for CRs
5a2eb43 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 14, 2024
@rexagod
Set resource version for CRs
dbef16f 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 14, 2024
@rexagod
Set resource version for CRs
45003c5 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 15, 2024
@rexagod
Set resource version for CRs
f7f48cc 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 16, 2024
@rexagod
Set resource version for CRs
3cd47e3 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 17, 2024
@rexagod
Set resource version for CRs
9c3a79d 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 17, 2024
@rexagod
Set resource version for CRs
906bf08 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 17, 2024
@rexagod
Set resource version for CRs
17a435d 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 17, 2024
@rexagod
Set resource version for CRs
eff369e 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 17, 2024
@rexagod
Set resource version for CRs
8c680f3 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 17, 2024
@rexagod
Set resource version for CRs
9efe8f5 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/library-go that referenced this pull request Oct 17, 2024
@rexagod
Set resource version for CRs
fc123b2 
Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/cluster-monitoring-operator that referenced this pull request Jan 29, 2025
@rexagod
OCPBUGS-17113: Lazy updates for Prometheus
37abec5 
Incorporate openshift/library-go#1575
downstream.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod added a commit to rexagod/cluster-monitoring-operator that referenced this pull request Jan 29, 2025
@rexagod
OCPBUGS-17113: Lazy updates for Prometheus
39addc0 
Incorporate openshift/library-go#1575
downstream.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jan--f jan--f jan--f left review comments

@p0lyn0mial p0lyn0mial p0lyn0mial left review comments

@dusk125 dusk125 dusk125 requested changes

@dgrisonnet dgrisonnet Awaiting requested review from dgrisonnet

@juzhao juzhao Awaiting requested review from juzhao

@deads2k deads2k Awaiting requested review from deads2k

@machine424 machine424 Awaiting requested review from machine424

Assignees

@jan--f jan--f

@dusk125 dusk125

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@rexagod @openshift-ci-robot @dgrisonnet @dusk125 @jan--f @deads2k @p0lyn0mial @machine424 @openshift-merge-robot