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

add new config-reloader image #720

Merged

Conversation

jbiers
Copy link
Member

@jbiers jbiers commented Sep 5, 2024

Pull Request Checklist

  • Change does not remove any existing Images or Tags in the images-list file
  • Change does not remove / overwrite exiting Images or Tags in Rancher DockerHub
  • If updating an existing entry, verify the SOURCE is still accurate and upstream hasn't been migrated to a new regitry or repo (if they've migrated, a new repo request to EIO is needed to comply with the SOURCE DESTINATION TAG pattern)
  • New entries are in format SOURCE DESTINATION TAG
  • New entries are added to the correct section of the list (sorted lexicographically)
  • New entries have a repo created in Rancher Dockerhub (where the image will be mirrored to)
  • New entries are licensed with Rancher favored licenses - Apache 2 and MIT - or approved licenses - as according to CNCF approved licenses.
  • New entries, when used in Rancher or Rancher's provided charts, have their corresponding origin added in Rancher's images origins file (must be added for all Rancher versions >= v2.7).
  • Changes to scripting or CI config have been tested to the best of your ability

Types of Change

New image to be used by rancher-logging.
The logging-operator has moved from https://github.com/jimmidyson/configmap-reload in favor of https://github.com/kube-logging/config-reloader, as can be seen here. We need to keep our image dependencies up to date with upstream.

Linked Issues

rancher/rancher#46924

Additional Notes

Final Checks after the PR is merged

  • Confirm that you can pull the new images and tags from DockerHub

@jbiers jbiers requested a review from a team as a code owner September 5, 2024 20:26
@brandond
Copy link
Member

brandond commented Sep 5, 2024

so now we're using two different images? ref: #718 / c72b02c
cc @mallardduck

@jbiers
Copy link
Member Author

jbiers commented Sep 5, 2024

so now we're using two different images? ref: c72b02c cc @mallardduck

The dependency @mallardduck added was for the monitoring chart, while this one is for logging. Probably each upstream project has moved to different directions regarding this project, but correct me if I'm mistaken ;)

@mallardduck
Copy link
Member

so now we're using two different images? ref: #718 / c72b02c cc @mallardduck

Yeah, so I totally get the confusion since they are very similar things in both name and function. However as @jbiers said, this one comes from upstream logging and the one from my PR comes from upstream monitoring (specifically for alerting drivers charts). It seems that logging used to use the same one, but then they switched to one they maintain.

It does make me wonder if we could converge onto using the same thing in both places, however given the trajectory of the monitoring project I'm not sure it's worth the dev/QA cycles to change that much.

@brandond
Copy link
Member

brandond commented Sep 7, 2024

Looks like you need to open an EIO ticket to create the new target repo. Make sure to ask for the bot account to have write access.

@jbiers jbiers force-pushed the migrate-configmap-reload-to-config-reloader branch from cbba65b to b8f1368 Compare September 16, 2024 21:06
@jbiers
Copy link
Member Author

jbiers commented Sep 16, 2024

@brandond the eio issue has now been resolved :)

@brandond brandond merged commit 7531385 into rancher:master Sep 16, 2024
1 check passed
@brandond
Copy link
Member

you might check and make sure the tag is successfully mirrored once CI finishes

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.

3 participants