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.

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

use directory-mapping instead of simple containerd.socket-file-mapping #633

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

devfaz
Copy link
Contributor

@devfaz devfaz commented Feb 22, 2024

What type of PR is this?

/kind bug

/kind chart-release

Any specific area of the project related to this PR?

/area falco-chart

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #632

Special notes for your reviewer:

to allow falco to reconnect if containerd got restarted on host

Checklist

  • Chart Version bumped
  • Variables are documented in the README.md
  • CHANGELOG.md updated

@poiana poiana added kind/bug Something isn't working dco-signoff: no kind/chart-release Add this label when the chart version has been bumped area/falco-chart labels Feb 22, 2024
@poiana
Copy link
Contributor

poiana commented Feb 22, 2024

Welcome @devfaz! It looks like this is your first PR to falcosecurity/charts 🎉

@alacuku
Copy link
Member

alacuku commented Mar 5, 2024

Hi @devfaz, thank you for the PR!

Could you please update the docs by running make docs?

@devfaz
Copy link
Contributor Author

devfaz commented Mar 5, 2024

Hi @devfaz, thank you for the PR!

Could you please update the docs by running make docs?

run make docs but it didnt generate any change.

@alacuku
Copy link
Member

alacuku commented Mar 5, 2024

Hi @devfaz, thank you for the PR!
Could you please update the docs by running make docs?

run make docs but it didnt generate any change.

Running make docs on your branch:

diff --git a/charts/falco/README.md b/charts/falco/README.md
index e83c3f5..81ecd66 100644
--- a/charts/falco/README.md
+++ b/charts/falco/README.md
@@ -581,7 +581,7 @@ If you use a Proxy in your cluster, the requests between `Falco` and `Falcosidek
 
 ## Configuration
 
-The following table lists the main configurable parameters of the falco chart v4.2.2 and their default values. See [values.yaml](./values.yaml) for full list.
+The following table lists the main configurable parameters of the falco chart v4.2.3 and their default values. See [values.yaml](./values.yaml) for full list.

@devfaz
Copy link
Contributor Author

devfaz commented Mar 5, 2024

im using podman, so had to adapt the Makefile a bit to get it working. Just a FYI in case someone googles this later.

diff --git a/Makefile b/Makefile
index 86f6097..621c855 100644
--- a/Makefile
+++ b/Makefile
@@ -29,6 +29,7 @@ docs-%:
        --rm \
        --workdir=/helm-docs \
        --volume "$$(pwd):/helm-docs" \
+       --userns=keep-id:uid=$$(id -u),gid=$$(id -g) \
        -u $$(id -u) \
        jnorwood/helm-docs:$(DOCS_IMAGE_VERSION) \
        helm-docs -c ./charts/$* -t ./README.gotmpl -o ./README.md

@poiana
Copy link
Contributor

poiana commented Jun 4, 2024

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

@devfaz
Copy link
Contributor Author

devfaz commented Jun 5, 2024

/remove-lifecycle stale

@devfaz devfaz requested a review from alacuku June 5, 2024 08:09
@alacuku
Copy link
Member

alacuku commented Jun 21, 2024

@devfaz, do you plan to update this PR by any chance?

@devfaz
Copy link
Contributor Author

devfaz commented Jun 21, 2024

@alacuku I already did? Do you miss anything?

@alacuku
Copy link
Member

alacuku commented Jun 21, 2024

You need to rebase and resolve the conflicts.

to allow falco to reconnect if containerd got restarted on host

Fixes falcosecurity#632

Signed-off-by: Fabian Zimmermann <dev.faz@gmail.com>
@devfaz
Copy link
Contributor Author

devfaz commented Jun 21, 2024

You need to rebase and resolve the conflicts.

Done, sorry havnt seen it.

@alacuku

Copy link
Member

@alacuku alacuku left a comment

Choose a reason for hiding this comment

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

/lgtm

@poiana
Copy link
Contributor

poiana commented Jun 21, 2024

LGTM label has been added.

Git tree hash: a1483f69a6ffb4fef0da57099b2b45a9d9a44a6f

@poiana
Copy link
Contributor

poiana commented Jun 21, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alacuku, devfaz

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

@poiana poiana merged commit 7527d0f into falcosecurity:master Jun 21, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/falco-chart dco-signoff: yes kind/bug Something isn't working kind/chart-release Add this label when the chart version has been bumped lgtm size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

falco will lose connection to containerd.socket if containerd on host gets restarted
3 participants