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

[#774] docs: Fix the metadata.annotations: Too long in install.md #853

Merged
merged 2 commits into from
May 10, 2023

Conversation

cchung100m
Copy link
Collaborator

What changes were proposed in this pull request?

Fix the metadata.annotations: Too long in install.md

Why are the changes needed?

Solve the error message encountered below when using kubectl apply -f

The CustomResourceDefinition "remoteshuffleservices.uniffle.apache.org" is invalid: metadata.annotations: Too long: must have at most 262144 bytes

Fix: #774

Does this PR introduce any user-facing change?

No.

How was this patch tested?

No.

@cchung100m
Copy link
Collaborator Author

Hi @xianjingfeng

Following issue #774, I would appreciate that if you can help to review this PR, thank you.

@jerqi jerqi changed the title [#774][MINOR] docs: fix the metadata.annotations: Too long in install.md [#774] docs: fix the metadata.annotations: Too long in install.md May 8, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 8, 2023

Codecov Report

Merging #853 (8388709) into master (6bd8d13) will increase coverage by 2.15%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #853      +/-   ##
============================================
+ Coverage     56.58%   58.73%   +2.15%     
- Complexity     2174     2175       +1     
============================================
  Files           327      307      -20     
  Lines         15977    13617    -2360     
  Branches       1262     1262              
============================================
- Hits           9040     7998    -1042     
+ Misses         6429     5184    -1245     
+ Partials        508      435      -73     

see 22 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jerqi
Copy link
Contributor

jerqi commented May 8, 2023

It seems better that we use create when we need to create operator and we use apply when we need to update. https://blog.csdn.net/daiqinge/article/details/103249260
@advancedxy @wangao1236 WDYT?

xianjingfeng
xianjingfeng previously approved these changes May 9, 2023
Copy link
Member

@xianjingfeng xianjingfeng left a comment

Choose a reason for hiding this comment

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

LGTM

@advancedxy
Copy link
Contributor

Thanks for your contribution. I noticed similar error.

However, I believe a better solution would be,

# create, cannot use apply here, see https://github.com/apache/incubator-uniffle/issues/774
kubectl create -f ${crd-yaml-file}

# update, make sure the crd-yaml-file is a complete CRD file.
kubectl replace -f ${crd-yaml-file}

kubect replace would replaces the CRD, which doesn't throw an exception if the CRD is already installed.

Refs:

  1. https://medium.com/pareture/kubectl-install-crd-failed-annotations-too-long-2ebc91b40c7d
  2. https://www.pcbaecker.com/posts/kubernetes-crd-annotation-too-long-fix/

Copy link
Contributor

@advancedxy advancedxy left a comment

Choose a reason for hiding this comment

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

See previous comment

@advancedxy
Copy link
Contributor

It seems better that we use create when we need to create operator and we use apply when we need to update. https://blog.csdn.net/daiqinge/article/details/103249260 @advancedxy @wangao1236 WDYT?

We cannot apply with the whole CRD file even if we are just updating as the CRD file is too large. See my previous comments for solutions.

@jerqi jerqi changed the title [#774] docs: fix the metadata.annotations: Too long in install.md [#774] docs: Fix the metadata.annotations: Too long in install.md May 9, 2023
@cchung100m
Copy link
Collaborator Author

Thanks for your contribution. I noticed similar error.

However, I believe a better solution would be,

# create, cannot use apply here, see https://github.com/apache/incubator-uniffle/issues/774
kubectl create -f ${crd-yaml-file}

# update, make sure the crd-yaml-file is a complete CRD file.
kubectl replace -f ${crd-yaml-file}

kubect replace would replaces the CRD, which doesn't throw an exception if the CRD is already installed.

Refs:

  1. https://medium.com/pareture/kubectl-install-crd-failed-annotations-too-long-2ebc91b40c7d
  2. https://www.pcbaecker.com/posts/kubernetes-crd-annotation-too-long-fix/

Hi @advancedxy Thank you for the prompt reply and the detailed suggestions.

Copy link
Contributor

@smallzhongfeng smallzhongfeng left a comment

Choose a reason for hiding this comment

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

LGTM.

@jerqi jerqi merged commit 194c604 into apache:master May 10, 2023
23 checks passed
@jerqi
Copy link
Contributor

jerqi commented May 10, 2023

Merged to master. Thanks all.

@cchung100m cchung100m deleted the annotationsTooLong branch May 10, 2023 14:21
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.

[DOCS] metadata.annotations: Too long
6 participants