Skip to content

Fix 'resources' type description#828

Merged
jyunmitch merged 1 commit intoentando:mainfrom
almartino:patch-1
Mar 27, 2024
Merged

Fix 'resources' type description#828
jyunmitch merged 1 commit intoentando:mainfrom
almartino:patch-1

Conversation

@almartino
Copy link
Contributor

Summary:

This pull request addresses the discrepancy in the type definition for the resources field in the microservice model. The current implementation uses an array type, which causes issues during deployment. The correct type should be a plain JSON object.

Details:

The ent CLI does not perform any validation on the microservice model during pack execution, leading to potential errors during deployment.

Examples:

Example 1:
{
      "name": "testms",
      "stack": "custom",
      "dbms": "postgresql",
      "healthCheckPath": "/api/health",
      "commands": {
        "build": "...",
        "run": "...",
        "pack": "..."
      },
      "version": "0.0.5",
      "resources": [  // valid, no errors during pack phase
        {"cpu": "100m"},
        {"memory": "128M"}
      ]
}
Example 2:
{
      "name": "testms",
      "stack": "custom",
      "dbms": "postgresql",
      "healthCheckPath": "/api/health",
      "commands": {
        "build": "...",
        "run": "...",
        "pack": "..."
      },
      "version": "0.0.5",
      "resources": {  // valid, no errors during pack phase
        "cpu": "100m",
        "memory": "128M"
      }
}

Both examples work during the pack phase. However, if the deployed bundle is installed with the microservice defined as in Example 1, the error "plugin/testms.yaml invalid [...]" appears when clicking the install button in AppBuilder. On the other hand, the deployed bundle with the microservice defined as in Example 2 will also fail:

The "cm" pod logs a null pointer exception due to the missing "storage" parameter in the "resources" object. Could this parameter be made optional? Some containers do not require storage.

NPE detail
java.lang.NullPointerException: null
	at java.base/java.util.regex.Matcher.getTextLength(Matcher.java:1770) ~[na:na]
	at java.base/java.util.regex.Matcher.reset(Matcher.java:416) ~[na:na]
	at java.base/java.util.regex.Matcher.<init>(Matcher.java:253) ~[na:na]
	at java.base/java.util.regex.Pattern.matcher(Pattern.java:1134) ~[na:na]
	at org.entando.kubernetes.validator.descriptor.PluginDescriptorValidator.validatePluginResourcePropOrThrow(PluginDescriptorValidator.java:381) ~[classes!/:7.3.0-SNAPSHOT]
	at org.entando.kubernetes.validator.descriptor.PluginDescriptorValidator.validatePluginResources(PluginDescriptorValidator.java:366) ~[classes!/:7.3.0-SNAPSHOT]
	at org.entando.kubernetes.validator.descriptor.BaseDescriptorValidator.lambda$validateOrThrow$0(BaseDescriptorValidator.java:91) ~[classes!/:7.3.0-SNAPSHOT]
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541) ~[na:na]

Changes:

  • Corrected the type of the resources field to a plain JSON object to ensure consistency and avoid deployment errors.

The correct type here is not an array type
but a plain json object.
@jyunmitch jyunmitch merged commit b79ea8c into entando:main Mar 27, 2024
@almartino almartino deleted the patch-1 branch March 27, 2024 14:02
@jyunmitch
Copy link
Collaborator

Thank you for pointing this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants