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

Legacy syslog bindings code cleanup #438

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

chombium
Copy link
Contributor

@chombium chombium commented Nov 29, 2023

Description

With the introduction of mTLS for Syslog Drains we have changed the internal structure of the bindings. In order to enable updating without downtime of the cloud controller, the syslog binding cache and the syslog agent, we have introduced two endpoints for reading the syslog binding in the old and new format. This PR cleans up the code of the legacy syslog bindings and removes the http calls to the old Syslog Binding Cache API endpoint. From now on, only the /v2/binding and /v2/aggregate-bindings API endpoints of the Syslog Binding Cache will be used.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing performed?

  • Unit tests
  • Integration tests
  • Acceptance tests

Checklist:

  • This PR is being made against the main branch, or relevant version branch
  • I have made corresponding changes to the documentation
  • I have added testing for my changes

This PR should be merged after merging the changes introduced with the this PR in the Cloud Controller

With the introduction of mTLS for Syslog Drains we have changed the
internal structure of the bindings. This PR cleans up the code of the
legacy syslog bindings and removes the http calls to the old Syslog
Binding Cache API endpoint. From now on, only the `/v2/binding` and
`/v2/aggregate-bindings` API endpoints of the Syslog Binding Cache will
be used.
Copy link
Member

@mkocher mkocher left a comment

Choose a reason for hiding this comment

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

Great to see things getting cleaned up. We have some environments that have older syslog-binding-caches that do not speak /v2. We'd like to merge this and then cut a v8 line of loggregator-agent-release.

We'd like to wait a week or two to cut the v8 line so that we can double check that we're not going to pull in a v8 release into places where it shouldn't go. We're also thinking that we'd like to move otel-collector out to its own release, which would also be a breaking change. Are you OK with waiting a little bit and merging this and the otel-collector split in the same release?

@chombium
Copy link
Contributor Author

chombium commented Dec 2, 2023

Hi, @mkocher thank you very much for the review and the approval.
I know that @acrmp had some doubts about the usage of the original Syslog Binding Cache we (myself, @fhambrec, @Benjamintf1 and @ctlong) have added the new /v2 endpoints when we implemented mutual TLS to transfer certificates to the Syslog Agent and. The other important thing was to avoid downtime during CF update.

I've adjusted only Syslog Agent as that is the only Syslog Binding Cache that I know of. If you have any other internal consumers, take your time to adjust them. There is no rush yo merge this. We still have to wait for the changes in the Cloud Controller to be merged qnd new capi release is cut and added in cf-deployment. Do the needful and we'll create a new release when we're ready.

@mkocher mkocher merged commit 6c78221 into cloudfoundry:main Jan 17, 2024
7 checks passed
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

2 participants