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

UPSTREAM: <carry>: garbage collector monitors syncing #108

Merged
merged 5 commits into from
Nov 9, 2022

Conversation

astefanutti
Copy link
Member

Adds the ability to update garbage collector monitors for integrators.

Required for kcp-dev/kcp#2112.

@ncdc
Copy link
Member

ncdc commented Oct 4, 2022

Are you going to do an AST generator for this?

@astefanutti
Copy link
Member Author

Are you going to do an AST generator for this?

Yes, just preferred that you have a look at kcp-dev/kcp@2e900e7 before going forward.

@astefanutti
Copy link
Member Author

@ncdc I've just added the AST-based generator. Let me know if there is more to do.

@ncdc
Copy link
Member

ncdc commented Oct 5, 2022

@astefanutti thanks! Could you please create commits in this order?

  1. The AST generator by itself
  2. The unmodified output of running the generator (compare to commit 94dc4f4)
  3. The modifications to the patch file (compare to commit bcfaeb0)

@astefanutti
Copy link
Member Author

@ncdc thanks a lot. I've just re-pushed with commits articulated according to your instructions. Let me know if that doesn't meet your expectations.

@ncdc
Copy link
Member

ncdc commented Oct 5, 2022

This is great! 1 last request: can you include the clusterName in any/all log messages & errors? See e.g.

klog.V(2).Infof("%s: enqueuing resourcequota %s/%s because the list of available APIs changed", rq.clusterName, quota.Namespace, quota.Name)
where rq.clusterName comes from

@astefanutti
Copy link
Member Author

@ncdc I've just re-pushed with an extra commit that adds the cluster name to all log statements and errors.

@ncdc ncdc merged commit 522b775 into kcp-dev:feature-logical-clusters-1.24 Nov 9, 2022
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