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

OCPNODE-2098: Add static pod for kube-rbac-proxy-crio #4175

Merged

openshift-merge-bot merged 1 commit into openshift:master from rphillips:rbac_crio_metrics_static_pod

Mar 8, 2024

Contributor

rphillips commented Feb 7, 2024 •

edited

This PR adds a static pod to the MCO to lay down a kube-rbac-proxy to secure crio's metrics port. The TLS endpoints supports cert based authentication for monitoring.

Merge information

Allow static pod tolerations: 4.16/master : Backport to EUS 4.14
MCO Kube-Rbac-Proxy Static pods 4.16/master : OCPNODE-2098: Add static pod for kube-rbac-proxy-crio #4175 : Backport to EUS 4.14
Create an upgrade edge after all backports merge

After all the above backports are merged, then

Change the Monitoring Operator to use port 9637 (with client certificates) : OCPNODE-2100: jsonnet: update crio port to TLS port 9637 cluster-monitoring-operator#2257 : This PR is only for 4.16

This PR will need to be backported to EUS 4.14.

openshift-ci bot added the do-not-merge/work-in-progress label

openshift-ci bot requested review from cdoern and cgwalters

February 7, 2024 21:18

rphillips force-pushed the rbac_crio_metrics_static_pod branch 11 times, most recently from 16324c2 to 12bb710 Compare

February 8, 2024 20:14

rphillips mentioned this pull request

OCPNODE-2100: jsonnet: update crio port to TLS port 9637 openshift/cluster-monitoring-operator#2257

Merged

Contributor

openshift-ci bot commented Feb 8, 2024

@rphillips: This PR was included in a payload test run from openshift/cluster-monitoring-operator#2257
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.16-upgrade-from-stable-4.15-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/820f5280-c6d5-11ee-97cb-3e05c7124d92-0

Contributor

openshift-ci bot commented Feb 12, 2024

@rphillips: This PR was included in a payload test run from openshift/cluster-monitoring-operator#2257
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.16-upgrade-from-stable-4.15-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/896bb770-c93d-11ee-8788-320a5f153a65-0

rphillips force-pushed the rbac_crio_metrics_static_pod branch 5 times, most recently from d32a2a9 to 410e914 Compare

February 12, 2024 17:13

Contributor

openshift-ci bot commented Feb 12, 2024

@rphillips: This PR was included in a payload test run from openshift/cluster-monitoring-operator#2257
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.16-upgrade-from-stable-4.15-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/0d734920-c9ca-11ee-824a-026157b75804-0

rphillips mentioned this pull request

NO-JIRA: remove kube-rbac-proxy for crio port #4181

Merged

rphillips force-pushed the rbac_crio_metrics_static_pod branch from 410e914 to b8995e3 Compare

February 13, 2024 13:29

Contributor

openshift-ci bot commented Feb 13, 2024

@rphillips: This PR was included in a payload test run from openshift/cluster-monitoring-operator#2257
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.16-upgrade-from-stable-4.15-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/56f50e00-ca7c-11ee-974b-7cd8f62e7155-0

rphillips force-pushed the rbac_crio_metrics_static_pod branch 3 times, most recently from 08b787a to 5d99f07 Compare

February 13, 2024 18:44

Contributor

openshift-ci bot commented Feb 20, 2024

@rphillips: This PR was included in a payload test run from openshift/cluster-monitoring-operator#2257
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.16-upgrade-from-stable-4.15-e2e-aws-ovn-upgrade

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/71d442f0-d03e-11ee-95a3-a9525782e9b1-0

Contributor

openshift-ci-robot commented Mar 5, 2024 •

edited by openshift-ci bot

@rphillips: This pull request references OCPNODE-2098 which is a valid jira issue.

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

Contributor Author

rphillips commented Mar 5, 2024

/cherry-pick release-4.15

openshift-cherrypick-robot commented Mar 5, 2024

@rphillips: once the present PR merges, I will cherry-pick it on top of release-4.15 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.15

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.

Contributor Author

rphillips commented Mar 5, 2024

/hold until openshift/origin#28636 merges.

openshift-ci bot added the do-not-merge/hold label

rphillips changed the title ~~OCPNODE-2098: Add static pods for crio rbac proxy~~ OCPNODE-2098: Add static pod for kube-rbac-proxy-crio

Contributor

openshift-ci-robot commented Mar 5, 2024 •

edited by openshift-ci bot

@rphillips: This pull request references OCPNODE-2098 which is a valid jira issue.

In response to this:

description incoming...

This PR will need to be backported to EUS 4.14.

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.

rphillips mentioned this pull request

OCPNODE-2099: add kube-rbac-proxy-crio toleration change openshift/origin#28636

Merged

Contributor Author

rphillips commented Mar 6, 2024

/test e2e-aws-ovn

Contributor Author

rphillips commented Mar 6, 2024

/hold cancel

openshift/origin#28636 merged

openshift-ci bot removed the do-not-merge/hold label

Contributor

openshift-ci-robot commented Mar 6, 2024 •

edited by openshift-ci bot

@rphillips: This pull request references OCPNODE-2098 which is a valid jira issue.

In response to this:

This PR adds a static pod to the MCO to lay down a kube-rbac-proxy to secure crio's metrics port. The TLS endpoints supports cert based authentication for monitoring.

Merge information

Allow static pod tolerations (this PR): 4.16/master : Backport to EUS 4.14

MCO Kube-Rbac-Proxy Static pods 4.16/master : OCPNODE-2098: Add static pod for kube-rbac-proxy-crio #4175 : Backport to EUS 4.14

Create an upgrade edge after all backports merge

After all the above backports are merged, then

Change the Monitoring Operator to use port 9637 (with client certificates) : OCPNODE-2100: jsonnet: update crio port to TLS port 9637 cluster-monitoring-operator#2257 : This PR is only for 4.16

This PR will need to be backported to EUS 4.14.

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.

Contributor

openshift-ci-robot commented Mar 6, 2024 •

edited by openshift-ci bot

@rphillips: This pull request references OCPNODE-2098 which is a valid jira issue.

In response to this:

This PR adds a static pod to the MCO to lay down a kube-rbac-proxy to secure crio's metrics port. The TLS endpoints supports cert based authentication for monitoring.

Merge information

Allow static pod tolerations: 4.16/master : Backport to EUS 4.14

MCO Kube-Rbac-Proxy Static pods 4.16/master : OCPNODE-2098: Add static pod for kube-rbac-proxy-crio #4175 : Backport to EUS 4.14

Create an upgrade edge after all backports merge

After all the above backports are merged, then

Change the Monitoring Operator to use port 9637 (with client certificates) : OCPNODE-2100: jsonnet: update crio port to TLS port 9637 cluster-monitoring-operator#2257 : This PR is only for 4.16

This PR will need to be backported to EUS 4.14.

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.

harche reviewed

View reviewed changes

Contributor

harche left a comment

/lgtm

openshift-ci bot assigned harche

openshift-ci bot added the lgtm label

cdoern reviewed

View reviewed changes

Contributor

cdoern left a comment

some service account and port questions. but looks good!

templates/master/01-master-kubelet/_base/files/criometricsproxy.yaml

+                      ports:
+                      - containerPort: 9637
+                      args:
+                      - --secure-listen-address=:9637

Contributor

cdoern Mar 6, 2024

any specific reason for this port?

Contributor Author

rphillips Mar 6, 2024

The usual port is 9537. To make it easier to identify the secure port I used 9637 (100 more than the insecure one). (9637 is not in use by the /etc/services file).

templates/worker/01-worker-kubelet/_base/files/criometricsproxy.yaml

+                      - --logtostderr=true
+                      - --kubeconfig=/var/lib/kubelet/kubeconfig
+                      - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
+                      - --upstream=http://127.0.0.1:9537

Contributor

cdoern Mar 6, 2024

our kube-rbac-proxy ctrs usually use a different port, I am forgetting which. Should we mimic that here?

Contributor Author

rphillips Mar 6, 2024

The kube-rbac-proxy is exposing a host network port (9637) and forwarding to the host's 9537 port. I slightly deviated from the usual construct due to host networking.

templates/master/01-master-kubelet/_base/files/criometricsproxy-config.yaml

+                        path: /metrics
+                        verb: get
+                        user:
+                          name: system:serviceaccount:openshift-monitoring:prometheus-k8s

Contributor

cdoern Mar 6, 2024

is this using the openshift-monitoring SA?

In our other kube-rbac-proxy pods we specify which serviceAccountName: we want to use. Where is that happening in this PR?

Contributor Author

rphillips Mar 6, 2024

This new kube-rbac-proxy-crio service does not have access to service account secrets due to being a static pod. Instead, we are using client side certificate validation and only allowing the system:serviceaccount:openshift-monitoring:prometheus-k8s user.

Contributor

cdoern commented Mar 7, 2024

/approve
/lgtm

openshift-ci bot assigned cdoern

Contributor

openshift-ci bot commented Mar 7, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, harche, rphillips

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~~ [cdoern]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added the approved label

Contributor Author

rphillips commented Mar 7, 2024

/cherry-pick release-4.15

openshift-cherrypick-robot commented Mar 7, 2024

@rphillips: once the present PR merges, I will cherry-pick it on top of release-4.15 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.15

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.

Contributor

openshift-ci-robot commented Mar 7, 2024

/retest-required

Remaining retests: 0 against base HEAD 53bbe70 and 2 for PR HEAD 757befb in total

Contributor

openshift-ci bot commented Mar 7, 2024 •

edited

@rphillips: 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/okd-scos-e2e-aws-ovn	`757befb`	link	false	`/test okd-scos-e2e-aws-ovn`

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.

Contributor Author

rphillips commented Mar 8, 2024

/test e2e-gcp-op-single-node

openshift-merge-bot bot merged commit a073953 into openshift:master

16 of 17 checks passed

openshift-cherrypick-robot commented Mar 8, 2024

@rphillips: #4175 failed to apply on top of branch "release-4.15":

Applying: OCPNODE-2098: add static pods for rbacproxy
Using index info to reconstruct a base tree...
M	pkg/operator/sync.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/operator/sync.go
CONFLICT (content): Merge conflict in pkg/operator/sync.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OCPNODE-2098: add static pods for rbacproxy
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.15

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.

Contributor

openshift-bot commented Mar 8, 2024

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-machine-config-operator-container-v4.16.0-202403081913.p0.ga073953.assembly.stream.el8 for distgit ose-machine-config-operator.
All builds following this will include this PR.

rphillips mentioned this pull request

[release-4.15] OCPBUGS-30761: add static pods for rbacproxy #4252

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment