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

bug: missing make manifests and make docs-update before commit #590

Closed
antrema opened this issue Feb 27, 2023 · 2 comments · Fixed by #591
Closed

bug: missing make manifests and make docs-update before commit #590

antrema opened this issue Feb 27, 2023 · 2 comments · Fixed by #591

Comments

@antrema
Copy link
Collaborator

antrema commented Feb 27, 2023

Describe the issue

I found some discrepancies between code and CRDs description/Documentation:
code [apis/fluentd/v1alpha1/plugins/common/buffer_types.go]:

// The number of threads to flush/write chunks in parallel  
// +kubebuilder:validation:Pattern:="^\\d+$"  
FlushThreadCount *string `json:"flushThreadCount,omitempty"  

Whereas in CRDs
crds [charts/fluent-operator/crds/fluentd.fluent.io_clusteroutputs.yaml]:

flushThreadCount:
description: The sleep interval (seconds) for threads to
wait for the next flush try(when no chunks are waiting)
pattern: ^\d+$
type: string

To Reproduce

launch the commands:
make manifests
make docs-update

Expected behavior

No modification after launching these commands

Your Environment

- Fluent Operator version: #N/A
- Container Runtime: #N/A
- Operating system: #N/A
- Kernel version: #N/A

How did you install fluent operator?

#N/A

Additional context

#N/A

@benjaminhuo
Copy link
Member

benjaminhuo commented Feb 27, 2023

// The number of threads to flush/write chunks in parallel
// +kubebuilder:validation:Pattern:="^\d+$"
FlushThreadCount *string `json:"flushThreadCount,omitempty"

The following two files should be consistent with the code, the other docs like charts and manifests are all generated based on the following two files.
https://github.com/fluent/fluent-operator/blob/master/config/crd/bases/fluentd.fluent.io_clusteroutputs.yaml#L113
https://github.com/fluent/fluent-operator/blob/master/config/crd/bases/fluentd.fluent.io_outputs.yaml#L112

The cards in config/crd/bases should be automatically generated by running make generate which is included in make test, so users should run make test everytime they change the crd definition code. Users shouldn't change files in config/crd/bases manuallyright? @wanjunlei

https://github.com/fluent/fluent-operator/blob/master/Makefile#L54

@antrema
Copy link
Collaborator Author

antrema commented Feb 27, 2023

@benjaminhuo I didn't make the changes manually, I launched make manifests and make docs-update to generate the files from the code. All my changes have been obtained by above commands, not manually
EDIT: I think there was just an oversight in a previous commit/PR

benjaminhuo added a commit that referenced this issue Feb 27, 2023
Fixes #590 Update CRDs description / Documentation, conform to code
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 a pull request may close this issue.

2 participants