Add network policy for RKE2 Ingress Nginx #4300

dharmit · 2024-07-30T13:48:38Z

Issue:

Addresses issue raised in rancher/rancher#45603.

Problem

Prometheus target failing to come up

Solution

Using the modified chart in this PR enables the target to come up

Testing

Install rancher-monitoring-crd through this PR

Install rancher-monitoring using following Helm command:

helm install rancher-monitoring charts/rancher-monitoring/105.0.0-rc1+up57.0.3/ \
--namespace=cattle-monitoring-system \
--set alertmanager.alertmanagerSpec.configSecret=alertmanager-rancher-monitoring-alertmanager \
--set alertmanager.alertmanagerSpec.useExistingSecret=true \
--set global.cattle.clusterId=local \
--set global.cattle.clusterName=local \
--set global.cattle.rkePathPrefix="" \
--set global.cattle.rkeWindowsPathPrefix="" \
--set global.cattle.systemDefaultRegistry="" \
--set global.cattle.systemProjectId=p-cwh9d \
--set global.cattle.url=https://13.202.137.42.sslip.io \
--set global.systemDefaultRegistry="" \
--set prometheus.prometheusSpec.evaluationInterval=1m \
--set prometheus.prometheusSpec.retentionSize=50GiB \
--set prometheus.prometheusSpec.scrapeInterval=1m \
--set rke2ControllerManager.enabled=true \
--set rke2Etcd.enabled=true \
--set rke2IngressNginx.enabled=true \
--set rke2IngressNginx.networkPolicy.enabled=true \
--set rke2Proxy.enabled=true \
--set rke2Scheduler.enabled=true

Go to Rancher UI; Monitoring; Prometheus Targets
In the Prometheus UI search for the ServiceMonitor serviceMonitor/kube-system/rancher-monitoring-ingress-nginx/0. It should be UP.

Engineering Testing

Manual Testing

Automated Testing

QA Testing Considerations

Regressions Considerations

Backporting considerations

github-actions · 2024-07-30T13:48:48Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

dharmit · 2024-07-30T13:52:26Z

I have honestly got no clue about why there are so many lines modified in this PR. Steps I followed are below:

$ export PACKAGE=rancher-monitoring/rancher-monitoring
$ make prepare
// made changes
$ make patch
$ make charts

Did I do something wrong here? 🤔

alexandreLamarre

Rancher monitoring version needs to be bumped to 104.1.1-rc1. Recommend separating make patch into a separate commit from make charts for exactly this scenario, where you would to do something like rebase with drop.

Need to remove packages/rancher-monitoring/rancher-monitoring/generated-changes/overlay/crds as a bug in charts-build-scripts includes these by accident.

...nitoring/rancher-monitoring/generated-changes/overlay/crds/crds/crd-alertmanagerconfigs.yaml

github-actions · 2024-07-31T09:40:04Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

dharmit · 2024-07-31T09:41:56Z

@joshmeranda @alexandreLamarre thanks for your feedback, guys! I have modified as per your recommendations. Can you PTAL again? Thanks.

joshmeranda

Looking better, but git history could use some housekeeping. Can you update the commits to look closer to the steps outline here? Generally this will look like this:

export PACKAGE=rancher-monitoring/rancher-monitoring
make prepare
Make the changes you need to the prepare chart, in this case adding the network policy
make patch
Commit the changes to packages/rancher-monitoring/rancher-monitoring/generated-changes (don't forget to remove generated-changes/overlay/crds introduced by the bug)
Update the version in package.yaml
Commit changes to package.yaml
make charts
Commit changes made by make charts
Update and commit release.yaml with new version of chart

Following these steps will make it a good bit easier for the reviewer to see what was changed and why. Also make each commit smaller and prevents new chart versions from stalling our browsers when trying to load 100+ new files. Thanks!

dharmit · 2024-07-31T13:12:50Z

@joshmeranda I referred to the Making Changes To Packages section which mentions making version related changes first. In context of your comment, I followed steps 6-9 first and 1-5 after that.

If I follow the steps you suggested, it'd make changes to 104.1.0. And, AFAIK, 104.1.0 has already released, right? Correct me if I'm wrong because there's a good chance I am. :)

joshmeranda · 2024-07-31T13:18:16Z

@dharmit You're right that we don't want to make changes to 104.1.0 but since you won't run make charts until after you bump the the version in step 6/7 you shouldn't be updating any existing charts.

I keep forgetting that the docs recommend you update the version first. That's fine as long as its a separate commit. Generally I'll update package.yaml in its own commit since its easier to see the new version in a commit with 1 changed file rather than 100+ changes files, then run make charts at the end. This will still create the new charts at versions 104.1.1 since package.yaml will have the bumped version.

ericpromislow · 2024-07-31T20:37:45Z

index.yaml

+        - name: Chart Source
+          url: https://github.com/prometheus-community/helm-charts
+        - name: Upstream Project
+           url: https://github.com/prometheus-operator/kube-prometheus


There's an extra space before url:. Strictly speaking it's not a yaml error, as this block is a string, but it looks like it's intended to be input for some other yaml process, and therefore should be conforming yaml code

ericpromislow

Something in this process is generating broken yaml.

dharmit · 2024-08-02T09:05:05Z

I keep forgetting that the docs recommend you update the version first. That's fine as long as its a separate commit. Generally I'll update package.yaml in its own commit since its easier to see the new version in a commit with 1 changed file rather than 100+ changes files, then run make charts at the end. This will still create the new charts at versions 104.1.1 since package.yaml will have the bumped version.

I see your point. However, what I have done is again just as per the docs recommendation. It recommends updating the version in package.yaml, then doing make charts, and committing the changes.

I can totally do what you're recommending if that's the general practice followed by O&B team. Let me know.

joshmeranda · 2024-08-02T10:50:04Z

@dharmit Yeah that's totally fine to do it that way, its just less familiar. It makes it easier to ensure that the changes you made were brought into the generated chart. Feel free to continue!

But you'll still want to have separate commits for updating package.yaml, building the new charts at that version, making the necessary changes, and rebuilding the charts. The commit history for something like this pr should probably look like this:

Commit 1: bump rancher-monitoring/rancher-monitoring version to 104.1.1+up57.0.3
Commit 2: make charts
Commit 3: update release.yaml (can go anywhere as long as its updated`
Commit 4: Add network policy for RKE2 Nginx Ingress
Commit 5: 'make charts'

I get that this feels somewhat nitpicky, but because of the sheer amount of files that are updated in charts PRs it makes it much easier for reviewers to give a review.

github-actions · 2024-08-02T11:01:28Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

dharmit · 2024-08-02T11:02:56Z

I get that this feels somewhat nitpicky, but because of the sheer amount of files that are updated in charts PRs it makes it much easier for reviewers to give a review.

No, it doesn't sound nitpicky at all, TBH. Thanks a tonne for laying down the steps. I'll update the PR accordingly. 👍🏽

github-actions · 2024-08-05T08:12:25Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

github-actions · 2024-08-05T08:22:56Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

github-actions · 2024-08-05T12:45:38Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

github-actions · 2024-08-05T12:49:46Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

github-actions · 2024-08-05T13:22:13Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

joshmeranda · 2024-08-05T13:34:16Z

release.yaml

 fleet:
  - 104.0.1+up0.10.1-rc.1
+fleet-agent:


We should avoid adding references to charts that weren't updated in this PR. Remove the fleet-agent and rancher-vsphere-csi fields please. Otherwise LGTM!

Well, those changes are based on make check-release-yaml.

joshmeranda

see release.yaml comments above

mallardduck · 2024-08-13T13:37:31Z

I don't think you need my review here -- reviews from Josh and Alexandre are sufficient.

@ericpromislow - I think that because you left a "Requested Changes" review you must provide an "Approved" review before this PR will be passable. Ignore mine since I added them today intentionally for changes I suggested. But notice your's and alex...I think big picture we need a new PR even after mine and yours are approved since Alex can review currently.

joshmeranda · 2024-08-14T18:52:42Z

Seeing this upon install:

W0813 13:11:43.161908 44 warnings.go:70] spec.template.spec.containers[2].ports[0]: duplicate port definition with spec.template.spec.containers[1].ports[0]

@mallardduck This one is coming from upstream: https://github.com/grafana/helm-charts/blob/main/charts/grafana/templates/_pod.tpl#L1016-L1021

github-actions · 2024-08-16T06:56:16Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

github-actions · 2024-08-16T07:02:27Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

release.yaml

github-actions · 2024-08-16T10:47:31Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

dharmit · 2024-08-16T11:02:38Z

Not quite working out the box for me when I set to enabled. In the rancher installed apps section it thinks that it has network policies, but they 404 when I click their links.

@mallardduck IIUC (which is based on description in rancher/rancher#45603), you have to open Prometheus Targets from the Monitoring view and check if serviceMonitor/kube-system/rancher-monitoring-ingress-nginx is up. I'm not sure where in the Installed Apps would you see a Network Policy because I see none on my RKE2 setup. Also, please set up an RKE2 with CIS enabled. Below are the configuration files I used:

$ cat /etc/rancher/rke2/config.yaml
cni: calico
write-kubeconfig-mode: "0644"
profile: cis
pod-security-admission-config-file: /etc/rancher/rke2/rke2-custom-pss.yaml

$ cat /etc/rancher/rke2/rke2-custom-pss.yaml
apiVersion: apiserver.config.k8s.io/v1
kind: AdmissionConfiguration
plugins:
- name: PodSecurity
  configuration:
    apiVersion: pod-security.admission.config.k8s.io/v1beta1
    kind: PodSecurityConfiguration
    defaults:
      enforce: "restricted"
      enforce-version: "latest"
      audit: "restricted"
      audit-version: "latest"
      warn: "restricted"
      warn-version: "latest"
    exemptions:
      usernames: []
      runtimeClasses: []
      namespaces: [kube-system, cis-operator-system, tigera-operator, cattle-system, cattle-fleet-system, cattle-fleet-local-system, cattle-monitoring-system]

github-actions · 2024-08-19T15:30:35Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

github-actions · 2024-08-21T06:27:00Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

github-actions · 2024-09-02T08:34:41Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

Signed-off-by: Dharmit Shah <[email protected]>

github-actions · 2024-09-25T06:25:12Z

Validation steps

Ensure all container images have repository and tag on the same level to ensure that all container images are included in rancher-images.txt which are used by airgap customers.

  Ex:-
    longhorn-controller:
      repository: rancher/hardened-sriov-cni
      tag: v2.6.3-build20230913

Add a 👍 (thumbs up) reaction to this comment once done. CI won't pass without this reaction to the github-action bot's latest validation comment.
Approve the PR to run the CI check.

mallardduck · 2024-09-25T20:59:29Z

Edit: Disregard original message for now - Prachi can dismiss the blocking review.
Will review the PR first thing tomorrow and review again.

dharmit · 2024-09-26T05:21:43Z

Edit: Disregard original message for now - Prachi can dismiss the blocking review.

TBH, I have been backing up old branch and force pushing each time. So, in a sense, this PR keeps getting fresh each time. 😉

Will review the PR first thing tomorrow and review again.

Before you spend time reviewing, I want to get the thing about RC clarified. Last time I pinged you on Slack, we agreed upon following steps mentioned in #4404 (comment) for this PR. However, a recent Slack thread started by @joshmeranda has got me concluding that I don't need to do those and only have a PR with the actual changes. Did I get this right?

mallardduck · 2024-09-26T13:16:47Z

I don't need to do those and only have a PR with the actual changes. Did I get this right?

Yes that's correct - with the clarification we got from @joshmeranda, there's no need for this to bump RC.1 to RC.2.

OOO

dharmit requested a review from a team as a code owner July 30, 2024 13:48

alexandreLamarre previously requested changes Jul 30, 2024

View reviewed changes

joshmeranda requested changes Jul 30, 2024

View reviewed changes

...nitoring/rancher-monitoring/generated-changes/overlay/crds/crds/crd-alertmanagerconfigs.yaml Outdated Show resolved Hide resolved

dharmit force-pushed the fix-r/r-45603 branch from be72c19 to e82226e Compare July 31, 2024 09:39

joshmeranda requested changes Jul 31, 2024

View reviewed changes

ericpromislow reviewed Jul 31, 2024

View reviewed changes

ericpromislow requested changes Jul 31, 2024

View reviewed changes

dharmit force-pushed the fix-r/r-45603 branch from e82226e to ed2e1f6 Compare August 2, 2024 11:01

dharmit force-pushed the fix-r/r-45603 branch from ed2e1f6 to bb44425 Compare August 5, 2024 08:12

dharmit force-pushed the fix-r/r-45603 branch from bb44425 to fefa021 Compare August 5, 2024 08:22

dharmit force-pushed the fix-r/r-45603 branch from fefa021 to 8754e0c Compare August 5, 2024 12:45

dharmit force-pushed the fix-r/r-45603 branch from 8754e0c to d5a0ba0 Compare August 5, 2024 12:49

dharmit force-pushed the fix-r/r-45603 branch from d5a0ba0 to 9bd8422 Compare August 5, 2024 13:21

joshmeranda approved these changes Aug 5, 2024

View reviewed changes

joshmeranda requested changes Aug 5, 2024

View reviewed changes

mallardduck requested a review from alexandreLamarre August 13, 2024 13:34

dharmit force-pushed the fix-r/r-45603 branch from eff3af3 to 287a9e3 Compare August 16, 2024 06:56

dharmit force-pushed the fix-r/r-45603 branch from 287a9e3 to f0e4b0c Compare August 16, 2024 07:02

dharmit commented Aug 16, 2024

View reviewed changes

release.yaml Outdated Show resolved Hide resolved

dharmit force-pushed the fix-r/r-45603 branch from f0e4b0c to c59e209 Compare August 16, 2024 10:47

dharmit force-pushed the fix-r/r-45603 branch from c59e209 to aa9a6b0 Compare August 19, 2024 15:30

dharmit force-pushed the fix-r/r-45603 branch from 8944a71 to 1b30c2d Compare September 2, 2024 08:34

dharmit added 6 commits September 25, 2024 11:03

VERSION=105.1.0-rc.1+up61.3.2 CHART=rancher-monitoring-crd make remove

5b406d0

Signed-off-by: Dharmit Shah <[email protected]>

VERSION=105.1.0-rc.1+up61.3.2 CHART=rancher-monitoring make remove

d5b46f6

Signed-off-by: Dharmit Shah <[email protected]>

make prepare; make patch; make clean

7ee1301

Signed-off-by: Dharmit Shah <[email protected]>

bump version in package.yaml

1debeb2

Signed-off-by: Dharmit Shah <[email protected]>

make charts

01e9896

Signed-off-by: Dharmit Shah <[email protected]>

update release.yaml

62f24ba

Signed-off-by: Dharmit Shah <[email protected]>

dharmit force-pushed the fix-r/r-45603 branch from 1b30c2d to 62f24ba Compare September 25, 2024 06:25

mallardduck requested review from mallardduck and removed request for alexandreLamarre September 26, 2024 17:24

Add network policy for RKE2 Ingress Nginx #4300

Are you sure you want to change the base?

Add network policy for RKE2 Ingress Nginx #4300

Conversation

dharmit commented Jul 30, 2024 • edited Loading

Issue:

Problem

Solution

Testing

Engineering Testing

Manual Testing

Automated Testing

QA Testing Considerations

Regressions Considerations

Backporting considerations

github-actions bot commented Jul 30, 2024

Validation steps

dharmit commented Jul 30, 2024

alexandreLamarre left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 31, 2024

Validation steps

dharmit commented Jul 31, 2024

joshmeranda left a comment • edited Loading

Choose a reason for hiding this comment

dharmit commented Jul 31, 2024

joshmeranda commented Jul 31, 2024 • edited Loading

ericpromislow Jul 31, 2024

Choose a reason for hiding this comment

ericpromislow left a comment

Choose a reason for hiding this comment

dharmit commented Aug 2, 2024

joshmeranda commented Aug 2, 2024 • edited Loading

github-actions bot commented Aug 2, 2024

Validation steps

dharmit commented Aug 2, 2024

github-actions bot commented Aug 5, 2024

Validation steps

github-actions bot commented Aug 5, 2024

Validation steps

github-actions bot commented Aug 5, 2024

Validation steps

github-actions bot commented Aug 5, 2024

Validation steps

github-actions bot commented Aug 5, 2024

Validation steps

joshmeranda Aug 5, 2024

Choose a reason for hiding this comment

dharmit Aug 6, 2024

Choose a reason for hiding this comment

joshmeranda left a comment • edited Loading

Choose a reason for hiding this comment

mallardduck commented Aug 13, 2024 • edited Loading

joshmeranda commented Aug 14, 2024

github-actions bot commented Aug 16, 2024

Validation steps

github-actions bot commented Aug 16, 2024

Validation steps

github-actions bot commented Aug 16, 2024

Validation steps

dharmit commented Aug 16, 2024

github-actions bot commented Aug 19, 2024

Validation steps

github-actions bot commented Aug 21, 2024

Validation steps

github-actions bot commented Sep 2, 2024

Validation steps

github-actions bot commented Sep 25, 2024

Validation steps

mallardduck commented Sep 25, 2024 • edited Loading

dharmit commented Sep 26, 2024

mallardduck commented Sep 26, 2024

dharmit commented Jul 30, 2024 •

edited

Loading

alexandreLamarre left a comment •

edited

Loading

joshmeranda left a comment •

edited

Loading

joshmeranda commented Jul 31, 2024 •

edited

Loading

joshmeranda commented Aug 2, 2024 •

edited

Loading

joshmeranda left a comment •

edited

Loading

mallardduck commented Aug 13, 2024 •

edited

Loading

mallardduck commented Sep 25, 2024 •

edited

Loading