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 quota to endpoint status response #17877

Merged
merged 1 commit into from May 3, 2024
Merged

Conversation

tjungblu
Copy link
Contributor

@tjungblu tjungblu commented Apr 25, 2024

This adds the configured backend quota bytes to the endpoint status response. related discussion in #17872

This adds the configured backend quota bytes to the endpoint status response.
related discussion in etcd-io#17821

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
@tjungblu
Copy link
Contributor Author

seems the robustness is failing on cert issues from alexellis/arkade#1056

@alexellis
Copy link
Contributor

If the test is failing, could it be due to trying to download vmmeter or something else via arkade?

The arkade-get action can be used instead of curl | sh..

@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2024

Overall looks good to me. The change is clear and straightforward!

Thanks @tjungblu

@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2024

Link to #17872

@alexellis
Copy link
Contributor

From (uses curl/sh and the utility domain)

- uses: alexellis/setup-arkade@master

=>

To (Downloads directly from GitHub's releases page)

      - uses: alexellis/arkade-get@master
        with:
           vmmeter: latest

Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr
Copy link
Member

ahrtr commented Apr 30, 2024

We also need to change the client side, including etcdctl. It can be in a separate PR.

@tjungblu
Copy link
Contributor Author

I think Scott had this in #17871

Shall we also backport this?

@ahrtr
Copy link
Member

ahrtr commented Apr 30, 2024

Shall we also backport this?

Usually we don't backport new feature, but I am not against it if other members/contributors agree to backport it.

We need to ensure it doesn't break any existing client, for example an application which depends on the old etcd client SDK can still work with a new etcdserver.

@tjungblu
Copy link
Contributor Author

it is not critical for us to have in 3.5, but it's also not a big and invasive change.

for example an application which depends on the old etcd client SDK can still work with a new etcdserver.

Wouldn't protobuf just flag this as an unknown field?

@ahrtr
Copy link
Member

ahrtr commented Apr 30, 2024

Wouldn't protobuf just flag this as an unknown field?

For this PR, it's safe. My point is that we don't have such mix-version test cases,

  • new client SDK version vs old etcdserver
  • old client SDK version vs new etcdserver

It can be discussed separately.

@ahrtr
Copy link
Member

ahrtr commented May 1, 2024

cc @fuweid @jmhbnz @serathius

Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks @tjungblu

@ahrtr ahrtr merged commit 13c0f75 into etcd-io:main May 3, 2024
44 checks passed
@ahrtr
Copy link
Member

ahrtr commented May 3, 2024

@tjungblu Please update 3.6 changelog for now. thx. If we decide to backport the PR to 3.5 later, we can update the changelog for 3.5 (revert the changelog for 3.6) by then.

tjungblu added a commit to tjungblu/etcd that referenced this pull request May 6, 2024
change from etcd-io#17877

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
@tjungblu tjungblu deleted the 17872_status branch May 6, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants