Add additional core functions #48
Conversation
ChristianZaccaria
force-pushed
the
add-core-functions
branch
from
94ce10f
to
c8f7a50
Compare
support/core.go
Outdated
| func GetContainerName(t Test, container corev1.Container) string { | ||
| t.T().Helper() | ||
| t.Expect(container.Name).Should(gomega.Not(gomega.BeEmpty())) | ||
| return container.Name | ||
| } | ||
|
|
||
| func GetVolumeName(t Test, volume corev1.Volume) string { | ||
| t.T().Helper() | ||
| t.Expect(volume.Name).Should(gomega.Not(gomega.BeEmpty())) | ||
| return volume.Name | ||
| } | ||
|
|
||
| func GetServiceAccountName(t Test, serviceAccount corev1.ServiceAccount) string { | ||
| t.T().Helper() | ||
| t.Expect(serviceAccount.Name).Should(gomega.Not(gomega.BeEmpty())) | ||
| return serviceAccount.Name | ||
| } | ||
|
|
||
| func GetVolumeMountName(t Test, volumeMount corev1.VolumeMount) string { | ||
| t.T().Helper() | ||
| t.Expect(volumeMount.Name).Should(gomega.Not(gomega.BeEmpty())) | ||
| return volumeMount.Name | ||
| } | ||
|
|
||
| func GetEnvVarName(t Test, envVar corev1.EnvVar) string { | ||
| t.T().Helper() | ||
| t.Expect(envVar.Name).Should(gomega.Not(gomega.BeEmpty())) | ||
| return envVar.Name | ||
| } |
There was a problem hiding this comment.
Some of it can be replaced by:
func ResourceName(meta metav1.Object) string {
return meta.GetName()
}
There was a problem hiding this comment.
Just a note - for such helper functions I usually don't put Get prefix as the function doesn't query Kubernetes to get the object, just retrieving internal value.
There was a problem hiding this comment.
I don't quite understand how all those functions can be replaced by the func ResourceName(meta metav1.Object). They are of different type where we would be getting the name of the top-level resource such as a pod, as opposed to the embedded resources.
There was a problem hiding this comment.
Right, I didn't check that before. Unfortunately it seems they can't be replaced.
There was a problem hiding this comment.
We can attempt to replace all functions for:
func ResourceName(t Test, obj any) string {
t.T().Helper()
t.Expect(reflect.ValueOf(obj).Kind()).Should(gomega.Equal(reflect.Struct), "input must be a struct")
nameField := reflect.ValueOf(obj).FieldByName("Name")
return nameField.String()
}
ChristianZaccaria
force-pushed
the
add-core-functions
branch
from
c8f7a50
to
2a0e9f4
Compare
support/core.go
Outdated
| @@ -184,3 +185,10 @@ func GetNodeInternalIP(t Test, node corev1.Node) (IP string) { | |||
|
|
|||
| return | |||
| } | |||
|
|
|||
| func ResourceName(t Test, obj any) string { | |||
There was a problem hiding this comment.
AFAIK the function can have just one argument - the struct used to retrieve its field from.
Otherwise it can't be used in WithTransform function
There was a problem hiding this comment.
You're right! Thanks Karel - Fixed.
ChristianZaccaria
force-pushed
the
add-core-functions
branch
from
2a0e9f4
to
19c2d29
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ChristianZaccaria, sutaakar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 68eadc2
into
project-codeflare:main
Issue link
Jira: https://issues.redhat.com/browse/RHOAIENG-7487
What changes have been made
Added several core functions that are being used in this PR: project-codeflare/codeflare-operator#549
Verification steps
Tests should pass.
Checks