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.

Sign up for GitHub

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

Jump to bottom

chore: Update controller-tools #900

Merged
merged 5 commits into from
Mar 27, 2024
Merged

chore: Update controller-tools #900

merged 5 commits into from
Mar 27, 2024

Conversation

chrkl
Copy link
Contributor

@chrkl chrkl commented Mar 15, 2024 •
edited
Loading

Description

Previously used controller-tools 0.11.3 might have issues with latest Golang release.

Changes proposed in this pull request (what was done and why):

  • Update controller-tools to 0.14.0
  • Updated table-gen
  • Improve wording in CRD description

Changes refer to particular issues, PRs or documents:

Traceability

  • The PR is linked to a GitHub issue.
  • New features have a milestone set.
  • New features have defined acceptance criteria in a corresponding GitHub Issue, and all criteria are satisfied with this PR.
  • The corresponding GitHub issue has a respective area and kind label.
  • The follow-up issues (if any) are linked in the Related Issues section.
  • Adjusted the documentation if the change is user-facing.
  • The feature is unit-tested
  • The feature is e2e-tested

@chrkl chrkl requested review from a team as code owners March 15, 2024 13:02
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 15, 2024
@chrkl
Copy link
Contributor Author

chrkl commented Mar 15, 2024

/hold

@kyma-bot kyma-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 15, 2024
@chrkl
Copy link
Contributor Author

chrkl commented Mar 15, 2024

New controller-gen introduces new-lines in CRD docs, which breaks our generated Markdown tables. Fix in table-gen is required first.

NHingerl
NHingerl reviewed Mar 18, 2024
View reviewed changes
config/crd/bases/operator.kyma-project.io_telemetries.yaml Outdated
to define a custom amount of replicas to be used for
the gateway. Present only if Type = StaticScalingStrategyType.
description: |-
Static is a scaling strategy allowing you to define a custom amount of replicas to be used for the gateway. Present only if Type =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Static is a scaling strategy allowing you to define a custom amount of replicas to be used for the gateway. Present only if Type =
Static is a scaling strategy enabling you to define a custom amount of replicas to be used for the gateway. Present only if Type =

config/crd/bases/operator.kyma-project.io_telemetries.yaml Outdated
to define a custom amount of replicas to be used for
the gateway. Present only if Type = StaticScalingStrategyType.
description: |-
Static is a scaling strategy allowing you to define a custom amount of replicas to be used for the gateway. Present only if Type =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Static is a scaling strategy allowing you to define a custom amount of replicas to be used for the gateway. Present only if Type =
Static is a scaling strategy enabling you to define a custom amount of replicas to be used for the gateway. Present only if Type =

config/crd/bases/operator.kyma-project.io_telemetries.yaml
using the time when the API field changed is acceptable.
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This description is imported from a dependency and not under our control.

config/crd/bases/telemetry.kyma-project.io_logparsers.yaml
using the time when the API field changed is acceptable.
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.

config/crd/bases/telemetry.kyma-project.io_logpipelines.yaml
using the time when the API field changed is acceptable.
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.

config/crd/bases/telemetry.kyma-project.io_metricpipelines.yaml
using the time when the API field changed is acceptable.
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.

config/crd/bases/telemetry.kyma-project.io_tracepipelines.yaml
using the time when the API field changed is acceptable.
description: |-
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.

docs/user/resources/01-telemetry.md Outdated
| **conditions** | \[\]object | Conditions contain a set of conditionals to determine the State of Status.
If all Conditions are met, State is expected to be in StateReady. |
| **conditions.​lastTransitionTime** (required) | string | lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. |
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. |

docs/user/resources/02-logpipeline.md Outdated
| **conditions.​observedGeneration** | integer | observedGeneration represents the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. |
| **conditions.​reason** (required) | string | reason contains a programmatic identifier indicating the reason for the condition's last transition. Producers of specific condition types may define expected values and meanings for this field, and whether the values are considered a guaranteed API. The value should be a CamelCase string. This field may not be empty. |
| **conditions.​lastTransitionTime** (required) | string | lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. |
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. |

@chrkl chrkl mentioned this pull request Mar 25, 2024
Merged
@chrkl chrkl force-pushed the bump-controller-tools branch from 5adf714 to e77d464 Compare March 26, 2024 09:50
@kyma-bot kyma-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 26, 2024
@chrkl
Copy link
Contributor Author

chrkl commented Mar 26, 2024

/test pull-telemetry-manager-build

@kyma-bot kyma-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 26, 2024
@chrkl
Copy link
Contributor Author

chrkl commented Mar 27, 2024

/unhold

@kyma-bot kyma-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2024
@hisarbalik hisarbalik self-assigned this Mar 27, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Mar 27, 2024
@kyma-bot kyma-bot merged commit 1d2338b into kyma-project:main Mar 27, 2024
21 checks passed
@chrkl chrkl deleted the bump-controller-tools branch March 27, 2024 11:45
@a-thaler a-thaler added this to the 1.13.0 milestone Mar 27, 2024
@a-thaler a-thaler added area/dependency Dependency changes kind/chore Categorizes issue or PR as related to a chore. labels Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NHingerl NHingerl NHingerl approved these changes

@hisarbalik hisarbalik hisarbalik approved these changes

Assignees

@hisarbalik hisarbalik

@NHingerl NHingerl

Labels
area/dependency Dependency changes cla: yes Indicates the PR's author has signed the CLA. kind/chore Categorizes issue or PR as related to a chore. lgtm Looks good to me! size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.
5 participants
@chrkl @hisarbalik @NHingerl @a-thaler @kyma-bot