Adding a Runtime class resource #2080

sheneska · 2023-04-17T20:31:04Z

Description

Added a runtime class resource (with handler attribute only for now) & wrote test file that asserts for only the creating a runtime class step

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:

$ make testacc TESTARGS="-count 1 -run ^TestAccKubernetesruntime_class_v1_basic"

TF_ACC=1 go test "/Users/sacha/Documents/code/hashicorp/terraform-provider-kubernetes/kubernetes" -v -count 1 -run ^TestAccKubernetesruntime_class_v1_basic -timeout 3h
=== RUN   TestAccKubernetesruntime_class_v1_basic
--- PASS: TestAccKubernetesruntime_class_v1_basic (5.24s)
PASS
ok  	github.com/hashicorp/terraform-provider-kubernetes/kubernetes	6.160s

Release Note

Release note for CHANGELOG:

`resource/kubernetes_runtime_class_v1`: Add a new resource `kubernetes_runtime_class_v1`.

References

Fix: #1114

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

arybolovlev

Great job, @sheneska!

I have left a few comments regarding the acceptance tests. Once you address them, please make sure that they pass on your local cluster:

$ make testacc TESTARGS="-count 1 -run ^TestAccKubernetesruntime_class_v1_basic"

Please add the changelog file .changelog/2080.txt.

Let me know if you have any questions or need any assistance. Thanks!

kubernetes/resource_kubernetes_runtime_class_v1_test.go

BBBmau

Looks Good, just left some things to add such as Docs and adding pod tests

website/docs/r/runtime_class_v1.html.markdown

BBBmau · 2023-04-25T17:32:27Z

kubernetes/resource_kubernetes_runtime_class_v1_test.go

+  metadata {
+    name = %q
+  }
+  handler = "myclass"


Since we use the runtime class resource with pods, we should have a pod using a runtimeclass resource as part of the tests.

You don't need to create a Pod to actually test that this resource works. A RuntimeClass can exist without any Pods using it.

@jrhouston wouldn't we want to have a test that checks that it works as expected with a Pod? I left this suggestion since not having a proper handler value would result in the Pod not being created. We could always have a message that informs the user what values to use to prevent this from happening.

The schema already has validation for the handler field so they will get an error message if the handler value is not in the proper format.

Other than that the API server doesn't produce any error if you supply a handler that doesn't exist on the node. So we don't really have a way of informing them that they've specified a bad handler value.

jrhouston

Wondering about overhead and scheduling being missing – otherwise this looks good 🚀

jrhouston · 2023-05-03T19:31:06Z

kubernetes/resource_kubernetes_runtime_class_v1.go

+
+		Schema: map[string]*schema.Schema{
+			"metadata": metadataSchema("runtimeclass", true),
+


What about the overhead and scheduling fields? See API ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#runtimeclass-v1-node-k8s-io

Sacha had recommended that i focus on the handler at first and get it working perfectly, he also mentioned that since the handler is will be the most commonly used we should merge this first and then add overhead and scheduling afterwards

jrhouston · 2023-05-04T01:28:22Z

kubernetes/resource_kubernetes_runtime_class_v1.go

+				Type:         schema.TypeString,
+				Description:  "Specifies the underlying runtime and configuration that the CRI implementation will use to handle pods of this class",
+				Required:     true,
+				ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?`), ""),


Kubernetes actually has a utility function to validate RFC 1123 DNS labels: https://pkg.go.dev/k8s.io/apimachinery/pkg/util/validation#IsDNS1123Label

So we don't need to roll our own regex here.

…etes_runtime_class_v1 and runtime_class_v1 files

Done

sheneska requested a review from arybolovlev April 17, 2023 20:31

github-actions bot added documentation size/L size/XL and removed size/L labels Apr 17, 2023

arybolovlev previously requested changes Apr 19, 2023

View reviewed changes

arybolovlev self-requested a review April 21, 2023 13:14

sheneska requested a review from BBBmau April 24, 2023 17:27

sheneska marked this pull request as ready for review April 25, 2023 16:32

sheneska requested a review from a team as a code owner April 25, 2023 16:32

BBBmau previously requested changes Apr 25, 2023

View reviewed changes

jrhouston requested changes May 3, 2023

View reviewed changes

jrhouston reviewed May 4, 2023

View reviewed changes

sheneska force-pushed the runtime-class branch from 13c2a37 to 26b619a Compare May 4, 2023 14:30

jrhouston approved these changes May 4, 2023

View reviewed changes

sheneska added 11 commits May 4, 2023 13:17

WIP

23c1c44

Add a runtime class resource (only with handler attribute)

d950ec4

Add provider.go file

c04cfb1

Modify test cases in test file

497b27f

Modify the test file and created documentation file

1fa9218

Format tests in test file

abb587b

Add test cases for labels and annotations, cleaned up resource_kubern…

86cbd51

…etes_runtime_class_v1 and runtime_class_v1 files

Modify test file

262b8b2

Add changelog file

192e463

Modify test file

33e8dfd

Add example to docs page

1a0bc8a

sheneska force-pushed the runtime-class branch from 26b619a to 1a0bc8a Compare May 4, 2023 17:17

jrhouston removed the request for review from arybolovlev May 4, 2023 17:18

sheneska merged commit 73e2558 into main May 5, 2023

sheneska deleted the runtime-class branch May 5, 2023 13:43

github-actions bot locked as resolved and limited conversation to collaborators May 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding a Runtime class resource #2080

Adding a Runtime class resource #2080

sheneska commented Apr 17, 2023 •

edited by arybolovlev

Loading

arybolovlev left a comment

BBBmau left a comment

BBBmau Apr 25, 2023

jrhouston May 3, 2023

BBBmau May 4, 2023

jrhouston May 4, 2023

jrhouston left a comment

jrhouston May 3, 2023

sheneska May 3, 2023

jrhouston May 4, 2023


		Schema: map[string]*schema.Schema{
		"metadata": metadataSchema("runtimeclass", true),

Adding a Runtime class resource #2080

Adding a Runtime class resource #2080

Conversation

sheneska commented Apr 17, 2023 • edited by arybolovlev Loading

Description

Acceptance tests

Release Note

References

Community Note

arybolovlev left a comment

Choose a reason for hiding this comment

BBBmau left a comment

Choose a reason for hiding this comment

BBBmau Apr 25, 2023

Choose a reason for hiding this comment

jrhouston May 3, 2023

Choose a reason for hiding this comment

BBBmau May 4, 2023

Choose a reason for hiding this comment

jrhouston May 4, 2023

Choose a reason for hiding this comment

jrhouston left a comment

Choose a reason for hiding this comment

jrhouston May 3, 2023

Choose a reason for hiding this comment

sheneska May 3, 2023

Choose a reason for hiding this comment

jrhouston May 4, 2023

Choose a reason for hiding this comment

sheneska commented Apr 17, 2023 •

edited by arybolovlev

Loading