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

Support for Kubernetes >= 1.16 #1044

Merged
merged 4 commits into from
Apr 16, 2020
Merged

Support for Kubernetes >= 1.16 #1044

merged 4 commits into from
Apr 16, 2020

Conversation

maxgio92
Copy link
Member

@maxgio92 maxgio92 commented Feb 9, 2020

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area kubernetes

What this PR does / why we need it:

Support for Kubernetes 1.16 and 1.17 when deployed as DaemonSet.

Which issue(s) this PR fixes:

Fixes #1043
Fixes #993

Special notes for your reviewer:

Removed deprecated Kubernetes 1.16 API resource versions.

Affected API:

  • DaemonSet
  • Deployment
  • ClusterRole
  • ClusterRoleBinding

Documentation:

Does this PR introduce a user-facing change?:

NONE

Release note

docs(integrations): update API resource versions to Kubernetes 1.16

@poiana
Copy link

poiana commented Feb 9, 2020

Welcome @maxgio92! It looks like this is your first PR to falcosecurity/falco 🎉

@poiana poiana added the size/S label Feb 9, 2020
@maxgio92
Copy link
Member Author

maxgio92 commented Feb 9, 2020

Thanks @poiana!
/assign @leodido

krisnova
krisnova previously approved these changes Feb 11, 2020
Copy link
Contributor

@krisnova krisnova left a comment

Choose a reason for hiding this comment

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

Thank you! This was annoying me also.

@poiana
Copy link

poiana commented Feb 11, 2020

LGTM label has been added.

Git tree hash: df61ca37e4037f2f0fcd25c5e6d9d97692b6b407

@krisnova
Copy link
Contributor

/hold

The more I think about this the more I think we need to have this new chart in a different directory so that we can support both

@maxgio92
Copy link
Member Author

maxgio92 commented Feb 12, 2020

/hold

The more I think about this the more I think we need to have this new chart in a different directory so that we can support both

Thanks @kris-nova. Are you referrring to Kubernetes >= 1.16 and Kubernetes < 1.16?

@leodido
Copy link
Member

leodido commented Feb 25, 2020

/milestone 0.21.0

@poiana poiana added this to the 0.21.0 milestone Feb 25, 2020
@fntlnz
Copy link
Contributor

fntlnz commented Mar 5, 2020

Let me remove this from the milestone until we don't have a clear idea of how we want to do those. Also the integrations folder is not tied to a particular Falco release so we can do this whenever without a specific release for it. :)

@fntlnz fntlnz removed this from the 0.21.0 milestone Mar 5, 2020
@fntlnz
Copy link
Contributor

fntlnz commented Mar 8, 2020

@kris-nova do you want to hold this still? We might just ask @maxgio92 to create a different directory for this?

@fntlnz
Copy link
Contributor

fntlnz commented Mar 8, 2020

The change was already merged in the helm chart helm/charts#17339

@leodido
Copy link
Member

leodido commented Mar 12, 2020

@leogr this is the PR I was mentioning to you before

@leogr
Copy link
Member

leogr commented Mar 12, 2020

The change was already merged in the helm chart helm/charts#17339

Just an idea: we could automate this by using the helm chart to locally render the templates for each supported configuration. @leodido @fntlnz WDYT?

@fntlnz
Copy link
Contributor

fntlnz commented Apr 14, 2020

I think we can merge this and then have a discussion on how/where to keep manifests for old Kubernetes versions.

Also from the release announcement:

DaemonSet in the extensions/v1beta1 and apps/v1beta2 API versions is no longer served

    Migrate to use the apps/v1 API version, available since v1.9. Existing persisted data can be retrieved/updated via the new version.
    Notable changes:
        spec.templateGeneration is removed
        spec.selector is now required and immutable after creation; use the existing template labels as the selector for seamless upgrades
        spec.updateStrategy.type now defaults to RollingUpdate (the default in extensions/v1beta1 was OnDelete)

Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

Hey @maxgio92 we decided to merge this as soon the last changes requested by me and @fntlnz are committed.

In particular, this PR must also include the changes to replace deployment extensions/v1beta1 to apps/v1

integrations/k8s-using-deployment/k8s-with-rbac/falco-k8s-audit-deployment.yaml
1:apiVersion: extensions/v1beta1
integrations/k8s-using-deployment/falco-event-generator-deployment.yaml
1:apiVersion: extensions/v1beta1

Finally, we'd like help in maintaining these files (maybe for different K8S versions too). In case you're interested in helping us with this topic please join the next community call to make a plan together! :)

Massimiliano added 2 commits April 14, 2020 18:42
update API resource version and remove deprecated one.

Signed-off-by: maxgio92
<massimiliano.giovagnoli.1992@gmail.com>
update API resource version and remove deprecated one.

Signed-off-by: maxgio92
<massimiliano.giovagnoli.1992@gmail.com>
Massimiliano added 2 commits April 14, 2020 18:58
…version

replace extension/v1beta1 with 1.16-supported apps/v1 version as for release announcement

BREAKING CHANGE: spec.rollbackTo is removed, spec.selector is now required and immutable after
creation, spec.progressDeadlineSeconds now defaults to 600 seconds, spec.revisionHistoryLimit now
defaults to 10, maxSurge and maxUnavailable now default to 25%

issue #1043

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
replace rbac.authorization.k8s.io/v1beta1 with rbac.authorization.k8s.io/v1 as for the changelog

Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
@maxgio92
Copy link
Member Author

maxgio92 commented Apr 14, 2020

Thanks @leodido @fntlnz you're right, I only checked for the resources in the daemonset solution.
Now the manifests should be correct also for the deployment solution.

Why not? :) Sure!

@maxgio92
Copy link
Member Author

maxgio92 commented Apr 14, 2020

PS: I think that it depends on the level of automation in the whole project, but as for the installation steps other than Helm charts Kustomizations could be a starting point, with overlays for each main "breaking version", as it is integrated and minimal :)

@poiana poiana added the lgtm label Apr 15, 2020
@poiana
Copy link

poiana commented Apr 15, 2020

LGTM label has been added.

Git tree hash: 816f9ae80c2477ff3183138bdb08264d63749a3a

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

IMHO, we can now merge this, then 👉 #1145

@poiana
Copy link

poiana commented Apr 16, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leodido, leogr

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

@leodido
Copy link
Member

leodido commented Apr 16, 2020

/hold cancel

@poiana poiana merged commit 4d18203 into falcosecurity:master Apr 16, 2020
@leogr leogr added this to the 0.22.0 milestone Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to install Falco by applying DaemonSet Update the yaml files.
6 participants