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

MON-3211: Implement switching to metrics-server #2022

Merged
merged 6 commits into from Nov 24, 2023
Merged

MON-3211: Implement switching to metrics-server #2022

merged 6 commits into from Nov 24, 2023

Conversation

slashpai
Copy link
Member

@slashpai slashpai commented Jun 27, 2023 •
edited

As part of epic https://issues.redhat.com/browse/MON-3153

Note: Not all options of Prometheus Adapter are implemented in Metrics Server in this PR

(I will add ChangeLog once everything finalized to avoid rebase issues)

  • I added CHANGELOG entry for this change.
  • No user facing changes, so no entry in CHANGELOG was needed.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 27, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 27, 2023 •
edited by openshift-ci bot

@slashpai: This pull request references MON-3211 which is a valid jira issue.

In response to this:

As part of epic https://issues.redhat.com/browse/MON-3153

This is draft PR and not all intended things are completed

(I will add ChangeLog once everything finalized to avoid rebase issues)

  • I added CHANGELOG entry for this change.
  • No user facing changes, so no entry in CHANGELOG was needed.

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 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 27, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 27, 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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 27, 2023
@slashpai slashpai force-pushed the metrics-server branch 7 times, most recently from 276b86a to 8c3dfd5 Compare June 28, 2023 13:51
@slashpai
Copy link
Member Author

/test all

@slashpai
Copy link
Member Author

Most of the failures is due to image references which can done after updating https://github.com/openshift/kubernetes-metrics-server

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 3, 2023
@slashpai
Copy link
Member Author

/test all

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 19, 2023
@slashpai slashpai mentioned this pull request Jul 27, 2023
Closed
2 tasks
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2023
@slashpai
Copy link
Member Author

/test all

@slashpai
Copy link
Member Author

/test images

slashpai added a commit to slashpai/cluster-monitoring-operator that referenced this pull request Nov 23, 2023
@slashpai
MON-3505: move aggregated-metrics-reader role to cmo jsonnet
ce53aa6 
This change came as part of reviewing openshift#2022
To avoid duplicated code in metrics-server
and prometheus-adapter moving this role
to cluster-monitoring-operator.jsonnet

Signed-off-by: Jayapriya Pai <janantha@redhat.com>
jan--f
jan--f reviewed Nov 23, 2023
View reviewed changes
pkg/tasks/metricsserver.go Outdated Show resolved Hide resolved
@slashpai slashpai force-pushed the metrics-server branch 3 times, most recently from 2a29bb3 to 1d0948f Compare November 23, 2023 15:32
slashpai added a commit to slashpai/cluster-monitoring-operator that referenced this pull request Nov 23, 2023
@slashpai
MON-3505: move aggregated-metrics-reader role to cmo jsonnet
894009f 
This change came as part of reviewing openshift#2022
To avoid duplicated code in metrics-server
and prometheus-adapter moving this role
to cluster-monitoring-operator.jsonnet

Signed-off-by: Jayapriya Pai <janantha@redhat.com>
@slashpai
Copy link
Member Author

I will rebase once #2163 is merged. I thought that was merged as it was ready to merge state :(. CI seems to have issues

@slashpai
jsonnet: Add metrics-server component
ffb2d0e 
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
@slashpai
assets: regenerate
b63263e 
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
@slashpai
pkg/manifests: Deploy metrics-server through operator
89b2aa6 
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
@slashpai
pkg/{operator,tasks}: add featuregate checks for metrics-server
01f6f90 
docs: regenerate

Signed-off-by: Jayapriya Pai <janantha@redhat.com>
@slashpai
pkg/tasks: update prometheus-adapter remove method
83f6177 
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
@slashpai slashpai force-pushed the metrics-server branch 3 times, most recently from 7a0cc2f to cbe672b Compare November 24, 2023 05:33
@slashpai
Copy link
Member Author

cc: @juzhao

@slashpai
Copy link
Member Author

This is good for review again, addressed comments

simonpasquier
simonpasquier reviewed Nov 24, 2023
View reviewed changes
pkg/tasks/prometheusadapter.go Outdated Show resolved Hide resolved
pkg/operator/operator.go Outdated Show resolved Hide resolved
@jan--f
Copy link
Contributor

jan--f commented Nov 24, 2023

To me this looks good (pending Simons last comments). I'll leave the lgtm to him though.
/label acknowledge-critical-fixes-only

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Nov 24, 2023
@slashpai
pkg/{manifests,tasks}: update featuregate checks
f6e3155 
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
simonpasquier
simonpasquier reviewed Nov 24, 2023
View reviewed changes
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2023
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 24, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: simonpasquier, slashpai

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:
  • OWNERS [simonpasquier,slashpai]

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
Copy link
Contributor

openshift-ci bot commented Nov 24, 2023

@slashpai: 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/test-infra repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit f0b9123 into openshift:master Nov 24, 2023
17 checks passed
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build cluster-monitoring-operator-container-v4.15.0-202311241633.p0.gf0b9123.assembly.stream for distgit cluster-monitoring-operator.
All builds following this will include this PR.

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

@simonpasquier simonpasquier simonpasquier left review comments

@marioferh marioferh Awaiting requested review from marioferh

@rexagod rexagod Awaiting requested review from rexagod

Assignees

@simonpasquier simonpasquier

Labels
acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. 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.

None yet
6 participants
@slashpai @openshift-ci-robot @simonpasquier @jan--f @openshift-bot @openshift-merge-robot