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

Support JobBackoffLimitPerIndex feature gate fields #2421

Conversation

theloneexplorerquest
Copy link
Contributor

@theloneexplorerquest theloneexplorerquest commented Feb 10, 2024

Description

resolve #2398

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

└─(17:46:40 on Support-JobBackoffLimitPerIndex-feature-gate-fields)──> make testacc TESTARGS='-run TestAccKubernetesCronJobV1_minimalWithBackoffLimitPerIndex'
==> Checking that code complies with gofmt requirements...
go vet ./...
TF_ACC=1 go test "/Users/mau/Dev/terraform-provider-kubernetes/kubernetes" -v -vet=off -run TestAccKubernetesCronJobV1_minimalWithBackoffLimitPerIndex -parallel 8 -timeout 3h
=== RUN   TestAccKubernetesCronJobV1_minimalWithBackoffLimitPerIndex
=== PAUSE TestAccKubernetesCronJobV1_minimalWithBackoffLimitPerIndex
=== CONT  TestAccKubernetesCronJobV1_minimalWithBackoffLimitPerIndex
--- PASS: TestAccKubernetesCronJobV1_minimalWithBackoffLimitPerIndex (12.16s)
PASS
ok      github.com/hashicorp/terraform-provider-kubernetes/kubernetes   13.125s

Release Note

Release note for CHANGELOG:

Add `backoff_per_limit_index` and `max_failed_indexes` fields in `structure_job.go`

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@alexsomesan
Copy link
Member

Thanks a lot for the contribution, @theloneexplorerquest

We have a long standing policy of not including features still behind feature gates, because there is no mechanism available on the Kubernetes API for the provider to query whether the feature gate is on or off and it would lead to unexpected errors on clusters that don't have the feature on.

As I mentioned, we appreciate the contribution and would love to merge this once the feature becomes universally available. Please feel free to keep the PR up to date going forward.

Thanks!

@theloneexplorerquest
Copy link
Contributor Author

Hi @alexsomesan ,

Thanks for the update, I will keep this open and wait until this feature become universally available (and do more testing at that time!).

@theloneexplorerquest theloneexplorerquest marked this pull request as ready for review March 7, 2024 10:36
@theloneexplorerquest theloneexplorerquest requested a review from a team as a code owner March 7, 2024 10:36
@BBBmau BBBmau self-assigned this Aug 14, 2024
@BBBmau BBBmau added the no-documentation This issue or pull request does not have documentation changes. label Aug 14, 2024
@@ -57,6 +57,14 @@ func jobSpecFields(specUpdatable bool) map[string]*schema.Schema {
ValidateFunc: validateNonNegativeInteger,
Description: "Specifies the number of retries before marking this job failed. Defaults to 6",
},
"backoff_limit_per_index": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there's errors with the PR specifically due to the default value being used. Could you address these failures as well as include a test case for these new fields?

This is now supported with the feature gate being set to true by default. Let me know if there's anything unclear from adding tests.

 === NAME  TestAccKubernetesCronJobV1_minimalWithPodFailurePolicy
    resource_kubernetes_cron_job_v1_test.go:179: Step 1/2 error: Error running apply: exit status 1
        
        Error: CronJob.batch "tf-acc-test-jk4onc8cdw" is invalid: [spec.jobTemplate.spec.backoffLimitPerIndex: Invalid value: 6: requires indexed completion mode, spec.jobTemplate.spec.maxFailedIndexes: Invalid value: 6: requires indexed completion mode]
        
          with kubernetes_cron_job_v1.test,
          on terraform_plugin_test.tf line 1, in resource "kubernetes_cron_job_v1" "test":
           1: resource "kubernetes_cron_job_v1" "test" {

Copy link
Contributor

Choose a reason for hiding this comment

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

any help needed on this @theloneexplorerquest ?

Copy link
Contributor

Choose a reason for hiding this comment

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

just a heads-up I'll be wrapping this up to get it merged as this is part of our v2.33.0 milestone @theloneexplorerquest

@BBBmau BBBmau added this to the v2.23.0 milestone Aug 14, 2024
@github-actions github-actions bot added size/S and removed size/XS labels Sep 20, 2024
@github-actions github-actions bot added size/M and removed size/S labels Sep 21, 2024
Copy link
Contributor

@BBBmau BBBmau left a comment

Choose a reason for hiding this comment

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

everything is passing, they weren't prior due to needing to insure that this only runs on tests that have a cluster version of >1.29

@BBBmau BBBmau merged commit 9d6074c into hashicorp:main Sep 21, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-documentation This issue or pull request does not have documentation changes. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support JobBackoffLimitPerIndex feature gate fields
3 participants