cloudbreak icon indicating copy to clipboard operation
cloudbreak copied to clipboard

CB-18195 `BlueprintUtilV4Endpoint.createRecommendationByCredCrn` yields…

Open lajosrodek opened this issue 3 years ago • 1 comments
trafficstars

… bogus GW recommendation even if the blueprint explicitly contains KNOX_GATEWAY

  • Prefer the host group(s) of role KNOX_GATEWAY as the GATEWAY whenever possible, falling back to the former decision strategy based on host group name & cardinality in the lack of a proper KNOX_GATEWAY match.
    • A match in some host group is considered feasible only if its cardinality is positive. Matches with a zero cardinality are simply ignored.
    • In case of multiple feasible matches, the one with the smallest (positive) cardinality wins. If there are more than one such host groups, the choice among them is made based on the lexicographic order of their names (the smallest name picked from the ascending order).
  • Some code cleanup.
  • Testing:
    • Manual testing in local CB using DataEng HA 7.2.7, 7.2.8 and 7.2.15.
    • Added new UT and updated existing ones.

lajosrodek avatar Aug 16 '22 20:08 lajosrodek

swagger-compatibility-test errors:

CB_VERSION=2.60.0-b83 CB_TARGET_BRANCH=CB-2.60.0 ./scripts/swagger-check.sh
Tue Aug 16 20:18:34 UTC 2022
Swagger check

Determine previous version number based on current version (which is 2.60.0 from input 2.60.0-b83),
where major version number is: 2, minor: 60, patch: 0
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  3275  100  3275    0     0   5287      0 --:--:-- --:--:-- --:--:--  5282
100  3275  100  3275    0     0   5284      0 --:--:-- --:--:-- --:--:--  5282
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  3773  100  3773    0     0   5363      0 --:--:-- --:--:-- --:--:--  5359
100  3773  100  3773    0     0   5359      0 --:--:-- --:--:-- --:--:--  5359
Target branch for swagger check: CB-2.60.0
Downloading cloudbreak 2.60.0-b83 swagger definition, if possible https://cloudbreak-swagger.s3.eu-central-1.amazonaws.com/swagger-2.60.0-b83.json
change is compatible
2.59.0-b74
Downloading cloudbreak 2.59.0-b74 swagger definition, if possible https://cloudbreak-swagger.s3.eu-central-1.amazonaws.com/swagger-2.59.0-b74.json

================ COMPATIBILITY BREAKS in cloudbreak ================
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/Images" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/images)
  warnings.warn(
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/ImageStackDetails" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/images/properties/base-images/items/properties/stack-details)
  warnings.warn(
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/StackRepoDetails" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/images/properties/base-images/items/properties/stack-details/properties/repo)
  warnings.warn(
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/Versions" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/versions)
  warnings.warn(
ERROR rules:
	[MIS-E001] Delete Endpoint: Endpoint(http_verb=<HTTPVerb.GET: 'get'>, path='/v4/{workspaceId}/image_catalogs/image/type/{type}/provider/{provider}/runtime/{runtime}', operation=Operation(getImageFromDefaultWithRuntime)) (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E001.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/name/{name}/delete/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/name/{name}/get/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//autoscale/stack/crn/{crn}/get/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/delete/responses/200/schema/properties/responses/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/post/parameters/1/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/post/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/get/responses/200/schema/properties/responses/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v1/distrox/post/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/name/{name}/internal/get/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/stacks/{name}/cluster/put/parameters/2/schema/properties/hostgroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/{name}/request/get/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v1/distrox/name/{name}/get/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/stacks/post/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/crn/{crn}/delete/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/crn/{crn}/get/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v1/distrox/crn/{crn}/get/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/stacks/{name}/get/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/stacks/crn/{crn}/get/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/stacks/internal/post/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v1/distrox/internal/post/responses/200/schema/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/internal/post/parameters/2/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/internal/post/responses/200/schema/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//v4/{workspaceId}/recipes/internal/get/responses/200/schema/properties/responses/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//autoscale/stack/crn/{crn}/{userId}/cluster/put/parameters/2/schema/properties/hostgroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
===============================================================================

Downloading freeipa 2.60.0-b83 swagger definition, if possible https://freeipa-swagger.s3.us-east-2.amazonaws.com/swagger-2.60.0-b83.json
change is compatible
2.59.0-b74
Downloading freeipa 2.59.0-b74 swagger definition, if possible https://freeipa-swagger.s3.us-east-2.amazonaws.com/swagger-2.59.0-b74.json
change is compatible
Downloading environment 2.60.0-b83 swagger definition, if possible https://environment-swagger.s3.us-east-2.amazonaws.com/swagger-2.60.0-b83.json
change is compatible
2.59.0-b74
Downloading environment 2.59.0-b74 swagger definition, if possible https://environment-swagger.s3.us-east-2.amazonaws.com/swagger-2.59.0-b74.json
change is compatible
Downloading datalake 2.60.0-b83 swagger definition, if possible https://datalake-swagger.s3.us-east-2.amazonaws.com/swagger-2.60.0-b83.json
change is compatible
2.59.0-b74
Downloading datalake 2.59.0-b74 swagger definition, if possible https://datalake-swagger.s3.us-east-2.amazonaws.com/swagger-2.59.0-b74.json

================ COMPATIBILITY BREAKS in datalake ================
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/Images" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/images)
  warnings.warn(
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/ImageStackDetails" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/images/properties/base-images/items/properties/stack-details)
  warnings.warn(
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/StackRepoDetails" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/images/properties/base-images/items/properties/stack-details/properties/repo)
  warnings.warn(
/usr/lib/python3.8/site-packages/swagger_spec_validator/validator20.py:49: SwaggerValidationWarning: Found "$ref: #/definitions/Versions" with siblings that will be overwritten. See https://stackoverflow.com/a/48114924 for more information. (path #/definitions/CloudbreakImageCatalogV3/properties/versions)
  warnings.warn(
ERROR rules:
	[MIS-E002] Changed type: #/paths//sdx/{name}/detail/get/responses/200/schema/properties/stackV4Response/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
	[MIS-E002] Changed type: #/paths//sdx/crn/{clusterCrn}/detail/get/responses/200/schema/properties/stackV4Response/properties/instanceGroups/items/properties/recipes/items/properties (documentation: https://swagger-spec-compatibility.readthedocs.io/en/latest/rules/MIS-E002.html)
===============================================================================

Downloading redbeams 2.60.0-b83 swagger definition, if possible https://redbeams-swagger.s3.us-east-2.amazonaws.com/swagger-2.60.0-b83.json
change is compatible
2.59.0-b74
Downloading redbeams 2.59.0-b74 swagger definition, if possible https://redbeams-swagger.s3.us-east-2.amazonaws.com/swagger-2.59.0-b74.json
change is compatible

Incompatible changes:
- cloudbreak:2.59.0-b74
- datalake:2.59.0-b74
make: *** [swagger-compare] Error 1

These have nothing to do with my changes. :smiling_imp:

lajosrodek avatar Aug 16 '22 20:08 lajosrodek

integration-test failure in EnvironmentStartStopTest.testCreateStopStartEnvironment:

Cloudbreak await for flow EnvironmentTestDto[name: mock-test-52bc5f4141a940ad9d]: Test timed out, flow did not finish in time. Crn=crn:cdp:environments:us-west-1:cloudera:environment:51eec675-c1d8-4b43-9d84-84936dc97a44, FlowId=8c8ff918-29ee-4a8d-9d41-4bdf27a1bdd9, FlowChainId=null
com.sequenceiq.it.cloudbreak.exception.TestFailException: Test timed out, flow did not finish in time. Crn=crn:cdp:environments:us-west-1:cloudera:environment:51eec675-c1d8-4b43-9d84-84936dc97a44, FlowId=8c8ff918-29ee-4a8d-9d41-4bdf27a1bdd9, FlowChainId=null

SDX audit:

        "notificationType" : "STOPPED",
        "notification" : "Infrastructure successfully stopped",

No clue, retriggering the run.

lajosrodek avatar Aug 17 '22 08:08 lajosrodek

https://github.com/hortonworks/cloudbreak/pull/13251/files#diff-d5ff8f1ca39a8136bf39603552ca064a33c28be50b2379866d4d7ec4018aed75R278

    return instanceCounts.entrySet().stream()
            .filter(e -> e.getValue().getMinimumCount() <= 1)

shouldn't it be >=1? this was the cause why compute group was selected for gateway because the minimum node count is 0

sodre90 avatar Aug 17 '22 12:08 sodre90

@sodre90 That whole fallback logic is nonsense; there is no justification to choose one host group over the other one, as we are simply picking the first one in lexicographic order whose count matches the the above condition. I think the idea was to consider a group a GATEWAY if its count is at most 1; multi-node gateways were completely out of scope when this logic was written. As of now, only the DataEng HA has a 2-node GATEWAY (and of course the Heavy Duty DL, but that is a different matter anyway :smile: ). Actually, the <= here possibly could even be replaced with ==, but then there might be custom blueprints that did not have any host groups with a cardinality of exactly 1.

The choice of compute was indeed for the above reason as you mentioned, but only because it did not consider the presence of Knox in the blueprint at all. With the present change, this will not affect any blueprints (custom or built-in) as long as they do explicitly include Knox. As for the ones that do not, I do not see what condition regarding the cardinality would be perfect here. :disappointed:

lajosrodek avatar Aug 17 '22 13:08 lajosrodek

@lajosrodek I think >= 1 would be better because groups with 0 minimum counts are usually compute / executor groups, but we can do this in another commit.

sodre90 avatar Aug 18 '22 08:08 sodre90

Thanks, I will raise an improvement ticket for this! :smile:

lajosrodek avatar Aug 18 '22 08:08 lajosrodek