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

convert ruby deps to Gemfile #68

Merged
merged 18 commits into from
Oct 6, 2023
Merged

convert ruby deps to Gemfile #68

merged 18 commits into from
Oct 6, 2023

Conversation

24367dfa
Copy link
Contributor

@24367dfa 24367dfa commented Oct 4, 2023

closes #32, #57

i currently see conflicts in dependency resolution:

12.73 Could not find compatible versions
12.73
12.73 Because every version of kubeclient depends on http >= 3.0, < 6.0
12.73   and every version of fluent-plugin-logdna depends on http >= 2.0.3, < 3.A,
12.73   every version of kubeclient is incompatible with fluent-plugin-logdna >= 0.
12.73 So, because Gemfile depends on fluent-plugin-logdna = 0.4.0
12.73   and Gemfile depends on kubeclient >= 0,
12.73   version solving has failed.

But all gems are currentliy pinned to their exact versions as currently specified in Dockerfile.
Are there reasons not to update all gems to their latest versions? 0:-)

@pepov
Copy link
Member

pepov commented Oct 4, 2023

@24367dfa how about introducing a new 1.16 fluentd based image with the gemfile and everything upgraded to latest and deprecate the 1.15 image?
We could also do the gemfile based upgrade with 1.15, but I would do that in the staging image only to avoid breaking existing deployments accidentally.

@24367dfa
Copy link
Contributor Author

24367dfa commented Oct 4, 2023

@24367dfa how about introducing a new 1.16 fluentd based image with the gemfile and everything upgraded to latest and deprecate the 1.15 image? We could also do the gemfile based upgrade with 1.15, but I would do that in the staging image only to avoid breaking existing deployments accidentally.

That sounds like a reasonable plan 👍

/e: that didn't quite solve our problem. After downgrading kubeclient another conflict popped up :-/

Because fluent-plugin-aws-elasticsearch-service >= 2.3.0 depends on
faraday_middleware-aws-sigv4 ~> 0.3.0
and fluent-plugin-opensearch >= 1.1.0 depends on faraday_middleware-aws-sigv4
~> 1.0.1,
fluent-plugin-aws-elasticsearch-service >= 2.3.0 is incompatible with
fluent-plugin-opensearch >= 1.1.0.
So, because Gemfile depends on fluent-plugin-opensearch = 1.1.2
  and Gemfile depends on fluent-plugin-aws-elasticsearch-service = 2.4.1,
  version solving has failed.

@pepov
Copy link
Member

pepov commented Oct 4, 2023

We can simply call the image v1.16 instead of v1.16-staging

@24367dfa
Copy link
Contributor Author

24367dfa commented Oct 4, 2023

We can simply call the image v1.16 instead of v1.16-staging

/done

I still see incompatibilities. I suppose they were always here, switching to Gemfile simply made them visible. :-/

@pepov do you have a preference how to proceed?

  • remove gems (not really an option i suppose)
  • fix dependencies in conflicting gems (some of them were not touched since 2017)
  • another way i can't see yet?

@24367dfa 24367dfa marked this pull request as draft October 4, 2023 21:46
@24367dfa 24367dfa changed the title WIP: convert ruby deps to Gemfile convert ruby deps to Gemfile Oct 4, 2023
Signed-off-by: David Kilias <[email protected]>
Signed-off-by: David Kilias <[email protected]>
Signed-off-by: David Kilias <[email protected]>
v1.16/Gemfile Show resolved Hide resolved
v1.16/Gemfile Show resolved Hide resolved
v1.16/Gemfile Show resolved Hide resolved
@pepov
Copy link
Member

pepov commented Oct 5, 2023

For a while now I'm thinking about splitting up the image into multiple smaller one, however it has a few downsides:

  • the operator doesn't have any knowledge about what outputs an image supports
  • what if a user would want to use multiple different outputs, the operator doesn't support an architecture that handles multiple different images within a single logging resource
  • exploding the single image into tens of smaller images

A middle ground could be to support an image with only properly maintained dependencies and break out a few to support specific outputs, like logdna or even drop support where it is reasonable with aws elasticsearch for example.

@24367dfa
Copy link
Contributor Author

24367dfa commented Oct 5, 2023

For a while now I'm thinking about splitting up the image into multiple smaller one, however it has a few downsides:

* the operator doesn't have any knowledge about what outputs an image supports

* what if a user would want to use multiple different outputs, the operator doesn't support an architecture that handles multiple different images within a single logging resource

* exploding the single image into tens of smaller images

A middle ground could be to support an image with only properly maintained dependencies and break out a few to support specific outputs, like logdna or even drop support where it is reasonable with aws elasticsearch for example.

i would propose to limit this image to properly supported (criteria to be defined) outputs and provide the users documentation on how to install any outputs they are missing on top to create their custom fluentd image

@aslafy-z
Copy link
Contributor

aslafy-z commented Oct 5, 2023

Related kube-logging/logging-operator#198

@24367dfa
Copy link
Contributor Author

24367dfa commented Oct 5, 2023

Related kube-logging/logging-operator#198

i'd like to keep adding new plugins out of scope for this MR. my goal was to get a gemfile based docker build working, to start working off of.

@aslafy-z
Copy link
Contributor

aslafy-z commented Oct 5, 2023

Related kube-logging/logging-operator#198

i'd like to keep adding new plugins out of scope for this MR. my goal was to get a gemfile based docker build working, to start working off of.

Sure, it was not the point of my comment. Sorry for the confusion.

@24367dfa 24367dfa marked this pull request as ready for review October 5, 2023 20:23
@24367dfa
Copy link
Contributor Author

24367dfa commented Oct 5, 2023

The current state is a list of plugins, all compatible with each other. I sadly do not see a realistic way to resolve the conflicts.

This would mean, that we lose the following plugins:

  • fluent-plugin-aws-elasticsearch-service (no maintenance since 2021)
  • fluent-plugin-logdna (company rebranded/no active maintenance of plugin)
  • fluent-plugin-azure-storage-append-blob (abandoned by microsoft)

Do we need to remove them from the kube-logging fluentd docs?

@pepov
Copy link
Member

pepov commented Oct 6, 2023

The current state is a list of plugins, all compatible with each other. I sadly do not see a realistic way to resolve the conflicts.

This would mean, that we lose the following plugins:

  • fluent-plugin-aws-elasticsearch-service (no maintenance since 2021)
  • fluent-plugin-logdna (company rebranded/no active maintenance of plugin)
  • fluent-plugin-azure-storage-append-blob (abandoned by microsoft)

Do we need to remove them from the kube-logging fluentd docs?

No I can handle it. Thank you for your efforts so far, I'm happy to merge your config and my plan is:

  • create a base image (v1.16-base) from the new dockerfile you created. anyone can depend on this image and add their own output plugin if they like
  • use that image to build v1.16 with all the non-conflicting dependencies
  • update logging operator docs to highlight that the "dropped" plugins are only available in the old image
  • create a separate image for elastic v7 (that is a pain for multiple users, that elastic 7 is incompatible with the latest clients)

@pepov
Copy link
Member

pepov commented Oct 6, 2023

sorry, one correction: please change the azure blob storage plugin to the new fork and see if that is compatible

@24367dfa
Copy link
Contributor Author

24367dfa commented Oct 6, 2023

sorry, one correction: please change the azure blob storage plugin to the new fork and see if that is compatible

I already tried that, it conflicts with fluent-plugin-opensearch in one of its dependencies 😢

I think I'll open an issue on the fork and try to get them to make it compatible to the other plugins we are using.

/e: the issue has already beed raised: Azure/azure-storage-ruby#227

@pepov
Copy link
Member

pepov commented Oct 6, 2023

sorry, one correction: please change the azure blob storage plugin to the new fork and see if that is compatible

I already tried that, it conflicts with fluent-plugin-opensearch in one of its dependencies 😢

I think I'll open an issue on the fork and try to get them to make it compatible to the other plugins we are using.

That works for me, we can skip it until we get a response and can decide what to do with it later

@pepov
Copy link
Member

pepov commented Oct 6, 2023

Merging this now as is and will follow up with the plan outlined above. You can go on and use the latest v1.16 once the artifacts are built! @24367dfa thanks again and check out the project discord https://discord.gg/xz2DTRB83d if you want to follow up the discussion!

@pepov pepov merged commit e4178d7 into kube-logging:main Oct 6, 2023
4 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.

Convert ruby deps in the fluentd image to Gemfile to benefit from automatic upgrade recommendations
3 participants