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.

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

changes for deploying l7 log collector daemon set using operator for calient #1505

Merged
merged 2 commits into from
Oct 21, 2021
Merged

changes for deploying l7 log collector daemon set using operator for calient #1505

merged 2 commits into from
Oct 21, 2021

Conversation

penkeysuresh
Copy link
Member

@penkeysuresh penkeysuresh commented Sep 7, 2021 •
edited

Description

As of calico enterprise v3.10, we enable L7 logs manually by applying manifests. This UX is cumbersome and can be improved by using operator to deploy the l7 daemon set. This PR included the designed UX for the same.

  • added new resourceApplicationLayer, used for deploying l7 log collector daemonset
  • added controller and renderer and test case for both
  • updated go to v1.16 and GO_BUILD_VER to v0.56
  • added envoy-config.yaml needed for log collection, embed in binary

issue on jira : https://tigera.atlassian.net/browse/BPF-1380
docs change: https://github.com/tigera/calico-private/pull/3701

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

@marvin-tigera marvin-tigera added this to the v1.22.0 milestone Sep 7, 2021
@penkeysuresh penkeysuresh changed the title Envoy ds changes for deploying l7 log collector daemon set using operator for calient Sep 7, 2021
@doublek doublek modified the milestones: v1.22.0, v1.23.0 Sep 13, 2021
@penkeysuresh penkeysuresh marked this pull request as ready for review September 16, 2021 14:50
@mgleung mgleung assigned mgleung and unassigned mgleung Sep 16, 2021
@mgleung mgleung requested a review from tmjd September 16, 2021 17:30
@mgleung
Copy link
Contributor

mgleung commented Sep 16, 2021

Do we also need to add the relevant PSPs or SCCs for this? I think we might be able to get away with not doing the PSPs because those are being deprecated in some K8s v1.21 but I think OCP will need the SCCs still.

@penkeysuresh
Copy link
Member Author

penkeysuresh commented Sep 17, 2021 •
edited

couple of things that are missing in this after recent comments on design doc

  1. Adding a watcher for felix configuration.
  2. Adding a hash of for config map to daemon set, so that any change in config map starts the daemon set rolling.

I'll add them to the PR soon

tmjd
tmjd requested changes Sep 17, 2021
View reviewed changes
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

I'm still reviewing the render but wanted to go ahead and submit this set of comments.

api/v1/applicationlayer_types.go Outdated Show resolved Hide resolved
config/samples/operator_v1_applicationlayer.yaml Outdated Show resolved Hide resolved
pkg/apis/crd.projectcalico.org/v1/felixconfig.go Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller_test.go Show resolved Hide resolved
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
Brian-McM
Brian-McM requested changes Sep 17, 2021
View reviewed changes
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
Brian-McM
Brian-McM requested changes Sep 17, 2021
View reviewed changes
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
statik/files/envoy-config.yaml Outdated Show resolved Hide resolved
tmjd
tmjd requested changes Sep 17, 2021
View reviewed changes
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/common/statik/statikfiles.go Outdated Show resolved Hide resolved
Brian-McM
Brian-McM requested changes Oct 7, 2021
View reviewed changes
api/v1/applicationlayer_types.go Outdated Show resolved Hide resolved
api/v1/applicationlayer_types.go Outdated Show resolved Hide resolved
controllers/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller_test.go Outdated Show resolved Hide resolved
tmjd
tmjd reviewed Oct 14, 2021
View reviewed changes
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

I've commented or mostly resolved on the previous comments. I still want to give it another look though but just wanted to submit what I could for now.

pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
tmjd
tmjd reviewed Oct 19, 2021
View reviewed changes
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

We've obviously still got some things to work out from our slack discussion but here is what I currently see.

pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer/applicationlayer.go Outdated Show resolved Hide resolved
pkg/render/applicationlayer/applicationlayer_test.go Show resolved Hide resolved
tmjd
tmjd reviewed Oct 20, 2021
View reviewed changes
Copy link
Member

@tmjd tmjd left a comment •
edited

Choose a reason for hiding this comment

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

Two more test requests. Otherwise I think we'll be good to go.

pkg/controller/applicationlayer/applicationlayer_controller_test.go Show resolved Hide resolved
pkg/controller/applicationlayer/applicationlayer_controller_test.go Show resolved Hide resolved
tmjd
tmjd approved these changes Oct 20, 2021
View reviewed changes
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

LGTM though I think you need to rebase on master.

@penkeysuresh
adding ApplicationLayer CRD, update go to v1.16
27e58d1 
  * added new resource ApplicationLayer, used for deploying l7 log collector daeonset
  * added controller and renderer and test case for both
  * updated go to v1.16 and GO_BUILD_VER to v0.56
  * added envoy-config.yaml needed for log collection, embed in binary
@tmjd tmjd merged commit 9ab4b11 into tigera:master Oct 21, 2021
@penkeysuresh penkeysuresh deleted the envoy-ds branch October 21, 2021 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doublek doublek doublek left review comments

@mgleung mgleung mgleung left review comments

@tmjd tmjd tmjd approved these changes

@Brian-McM Brian-McM Brian-McM approved these changes

Assignees
No one assigned
Labels
docs-pr-required release-note-required
Projects
None yet
Milestone
v1.23.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@penkeysuresh @mgleung @doublek @tmjd @Brian-McM @marvin-tigera