chore(jmx): remove configuration for JMX server, service, and auth secret #823
Conversation
|
/build_test |
|
|
That's correct. It made sense a few years ago when Cryostat was in a much earlier state and didn't have Custom Targets (let alone discovery plugins and the Agent) that it should automatically discover itself as a connectable target. Nowadays I am not sure that it's really necessary, since it's quick and easy for the user to use the |
|
AHh that makes sense! Thanks! |
| jmxPort := int32(9095) | ||
| cr := r.NewCryostatV1Beta1() | ||
| cr.Spec.ServiceOptions = &operatorv1beta1.ServiceConfigList{ | ||
| CoreConfig: &operatorv1beta1.CoreServiceConfig{ | ||
| HTTPPort: &httpPort, | ||
| JMXPort: &jmxPort, |
There was a problem hiding this comment.
These would be good to keep since this file is designed to be used by the conversion webhook tests. We could add a test that verifies that a v1beta1 Cryostat CR with a custom JMX port successfully becomes a v1beta2 CR without one.
There was a problem hiding this comment.
There is already a test that exercises that, I think, but currently fails:
Summarizing 1 Failure:
[FAIL] Cryostat converting from v1beta2 to v1beta1 [It] core service
/home/work/workspace/cryostat-operator/api/v1beta1/cryostat_conversion_test.go:72
Cryostat converting from v1beta2 to v1beta1 core service
/home/work/workspace/cryostat-operator/api/v1beta1/cryostat_conversion_test.go:116
[FAILED] in [It] - /home/work/workspace/cryostat-operator/api/v1beta1/cryostat_conversion_test.go:72 @ 05/22/24 11:49:35.675
• [FAILED] [0.000 seconds]
Cryostat converting from v1beta2 to v1beta1 [It] core service
/home/work/workspace/cryostat-operator/api/v1beta1/cryostat_conversion_test.go:116
[FAILED] Expected
<*v1beta1.Cryostat | 0xc0004c6000>: {
TypeMeta: {Kind: "", APIVersion: ""},
ObjectMeta: {
Name: "cryostat",
GenerateName: "",
Namespace: "test",
SelfLink: "",
UID: "",
ResourceVersion: "",
Generation: 0,
CreationTimestamp: {
Time: 0001-01-01T00:00:00Z,
},
DeletionTimestamp: nil,
DeletionGracePeriodSeconds: nil,
Labels: nil,
Annotations: nil,
OwnerReferences: nil,
Finalizers: nil,
ManagedFields: nil,
},
Spec: {
Minimal: false,
TrustedCertSecrets: nil,
EventTemplates: nil,
EnableCertManager: true,
StorageOptions: nil,
ServiceOptions: {
CoreConfig: {
HTTPPort: 8080,
JMXPort: nil,
ServiceConfig: {
ServiceType: "NodePort",
Annotations: {
"my/custom": "annotation",
},
Labels: {
"my": "label",
"app": "somethingelse",
},
},
},
GrafanaConfig: nil,
ReportsConfig: nil,
StorageConfig: nil,
},
NetworkOptions: nil,
ReportOptions: nil,
MaxWsConnections: 0,
JmxCacheOptions: nil,
Resources: nil,
AuthProperties: nil,
SecurityOptions: nil,
SchedulingOptions: nil,
TargetDiscoveryOptions: nil,
JmxCredentialsDatabaseOptions: nil,
OperandMetadata: nil,
},
Status: {Conditions: nil, GrafanaSecret: "", ApplicationURL: ""},
}
to equal
<*v1beta1.Cryostat | 0xc00046de00>: {
TypeMeta: {Kind: "", APIVersion: ""},
ObjectMeta: {
Name: "cryostat",
GenerateName: "",
Namespace: "test",
SelfLink: "",
UID: "",
ResourceVersion: "",
Generation: 0,
CreationTimestamp: {
Time: 0001-01-01T00:00:00Z,
},
DeletionTimestamp: nil,
DeletionGracePeriodSeconds: nil,
Labels: nil,
Annotations: nil,
OwnerReferences: nil,
Finalizers: nil,
ManagedFields: nil,
},
Spec: {
Minimal: false,
TrustedCertSecrets: nil,
EventTemplates: nil,
EnableCertManager: true,
StorageOptions: nil,
ServiceOptions: {
CoreConfig: {
HTTPPort: 8080,
JMXPort: 9095,
ServiceConfig: {
ServiceType: "NodePort",
Annotations: {
"my/custom": "annotation",
},
Labels: {
"my": "label",
"app": "somethingelse",
},
},
},
GrafanaConfig: nil,
ReportsConfig: nil,
StorageConfig: nil,
},
NetworkOptions: nil,
ReportOptions: nil,
MaxWsConnections: 0,
JmxCacheOptions: nil,
Resources: nil,
AuthProperties: nil,
SecurityOptions: nil,
SchedulingOptions: nil,
TargetDiscoveryOptions: nil,
JmxCredentialsDatabaseOptions: nil,
OperandMetadata: nil,
},
Status: {Conditions: nil, GrafanaSecret: "", ApplicationURL: ""},
}
In [It] at: /home/work/workspace/cryostat-operator/api/v1beta1/cryostat_conversion_test.go:72 @ 05/22/24 11:49:35.675
How should I adjust it to pass here? It seems like this test suite expects it to be possible to 1:1 convert beta1 to and from beta2, but that's not true in this case.
There was a problem hiding this comment.
I think we follow this setup for CRD fields that got removed. We would test the path from beta1 with Svc configurations to default beta2.
There was a problem hiding this comment.
Also, would it make sense to split the test case "core service" to consider cases with and without JMXPort? That way, we can still test other service configurations both way (for JMXPort, only beta1->beta2).
There was a problem hiding this comment.
How about this? I think this does what Thuan suggested. There would be two v1beta1 core service CRs, one with a JMX port and one without. Both should resolve to the same v1beta2 CR. The conversion tests pass for me with this patch.
diff --git a/api/v1beta1/cryostat_conversion_test.go b/api/v1beta1/cryostat_conversion_test.go
index ccc9ad59..a0058def 100644
--- a/api/v1beta1/cryostat_conversion_test.go
+++ b/api/v1beta1/cryostat_conversion_test.go
@@ -84,6 +84,8 @@ func tableEntriesTo() []TableEntry {
(*test.TestResources).NewCryostatWithIngress),
Entry("minimal mode", (*test.TestResources).NewCryostatWithMinimalModeV1Beta1,
(*test.TestResources).NewCryostat),
+ Entry("core JMX port", (*test.TestResources).NewCryostatWithCoreSvcJMXPortV1Beta1,
+ (*test.TestResources).NewCryostatWithCoreSvc),
)
}
diff --git a/internal/test/conversion.go b/internal/test/conversion.go
index 86f61472..ee42357e 100644
--- a/internal/test/conversion.go
+++ b/internal/test/conversion.go
@@ -168,12 +168,10 @@ func (r *TestResources) NewCryostatWithEmptyDirSpecV1Beta1() *operatorv1beta1.Cr
func (r *TestResources) NewCryostatWithCoreSvcV1Beta1() *operatorv1beta1.Cryostat {
svcType := corev1.ServiceTypeNodePort
httpPort := int32(8080)
- jmxPort := int32(9095)
cr := r.NewCryostatV1Beta1()
cr.Spec.ServiceOptions = &operatorv1beta1.ServiceConfigList{
CoreConfig: &operatorv1beta1.CoreServiceConfig{
HTTPPort: &httpPort,
- JMXPort: &jmxPort,
ServiceConfig: operatorv1beta1.ServiceConfig{
ServiceType: &svcType,
Annotations: map[string]string{
@@ -189,6 +187,13 @@ func (r *TestResources) NewCryostatWithCoreSvcV1Beta1() *operatorv1beta1.Cryosta
return cr
}
+func (r *TestResources) NewCryostatWithCoreSvcJMXPortV1Beta1() *operatorv1beta1.Cryostat {
+ jmxPort := int32(9095)
+ cr := r.NewCryostatWithCoreSvcV1Beta1()
+ cr.Spec.ServiceOptions.CoreConfig.JMXPort = &jmxPort
+ return cr
+}
+
func (r *TestResources) NewCryostatWithGrafanaSvcV1Beta1() *operatorv1beta1.Cryostat {
svcType := corev1.ServiceTypeNodePort
httpPort := int32(8080)
andrewazores
force-pushed
the
remove-jmx-auth
branch
from
9442651 to
0a32f9f
Compare
andrewazores
force-pushed
the
remove-jmx-auth
branch
from
eac6568 to
36beeb5
Compare
|
/build_test |
|
|
Welcome to Cryostat! 👋
Before contributing, make sure you have:
mainbranch[chore, ci, docs, feat, fix, test]git commit -S -m "YOUR_COMMIT_MESSAGE"Fixes: #822
Description of the change:
This change adds allows the users to provide...
Motivation for the change:
This change is helpful because users may want to...
How to manually test: