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

downgrade falco helm chart to 1.1.0 #457

Merged
merged 7 commits into from
Mar 4, 2020
Merged

Conversation

vsinghal13
Copy link
Contributor

@vsinghal13 vsinghal13 commented Mar 2, 2020

Description

This PR downgrade falco helm chart to 1.1.0 which points to falco image tag 0.18.0 for resolving the precompiled kernel modules error.

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in

@rvmiller89
Copy link
Contributor

rvmiller89 commented Mar 2, 2020

Is this recommended with Helm, or is it better practice to use the image provided in the Helm chart (whose subchart dependency is already pinned in requirements.yaml)?

@vsinghal13
Copy link
Contributor Author

Is this recommended with Helm, or is it better practice to use the image provided in the Helm chart (whose subchart dependency is already pinned in requirements.yaml)?

This is done as a work-around for now to get the precompiled kernel modules. We would have downgrade the falco helm chart version to get this particular app version and then upgrade it again later on.

With this change, we can override the image tag and still keep the helm chart version intact.

@rvmiller89
Copy link
Contributor

With this change, we can override the image tag and still keep the helm chart version intact.

Not opposed to this, but why not downgrade the helm chart version then?

@perk-sumo
Copy link
Contributor

With this change, we can override the image tag and still keep the helm chart version intact.

Not opposed to this, but why not downgrade the helm chart version then?

That is an excellent point by @rvmiller89 - I think we should just downgrade the whole falco chart to 1.1.0
Otherwise we could run into a problem when Docker image does not fit the helm chart.

@vsinghal13
Copy link
Contributor Author

@rvmiller89 @perk-sumo Thanks for the inputs. Modified it to use the older helm chart version.

@@ -8,7 +8,7 @@ dependencies:
repository: https://kubernetes-charts.storage.googleapis.com/
condition: prometheus-operator.enabled
- name: falco
version: 1.1.1
version: 1.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

should also update the dependency table in master branch so it gets included when we cut the next release?

Copy link
Contributor

@rvmiller89 rvmiller89 left a comment

Choose a reason for hiding this comment

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

Recommend updating the PR title and description to avoid confusion in the future

@vsinghal13 vsinghal13 changed the title pin falco image tag downgrade falco helm chart to 1.1.0 Mar 3, 2020
Copy link
Contributor

@perk-sumo perk-sumo left a comment

Choose a reason for hiding this comment

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

LGTM

@vsinghal13 vsinghal13 merged commit dbbc480 into master Mar 4, 2020
@vsinghal13 vsinghal13 deleted the vsinghal-pin-falco-image-tag branch March 4, 2020 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants