Skip to content

MINIFICPP-1596 Refactor docker tests by removing unnecessary clusters#1121

Closed
lordgamez wants to merge 22 commits intoapache:mainfrom
lordgamez:MINIFICPP-1596
Closed

MINIFICPP-1596 Refactor docker tests by removing unnecessary clusters#1121
lordgamez wants to merge 22 commits intoapache:mainfrom
lordgamez:MINIFICPP-1596

Conversation

@lordgamez
Copy link
Contributor

@lordgamez lordgamez commented Jun 29, 2021

Issues addressed in this PR regarding integration tests:

  • The main problem was the misuse of the Cluster concept in the codebase causing several issues
  • Cluster and Container concepts were mixed up in the codebase which made understanding the test code harder
  • In several test scenarios multiple clusters were created with the same network, which made the cleanup cumbersome and fail transiently
  • Some container deployments required creating new named clusters that were unnecessary and made the test scenario descriptions more complex
  • The SingleNodeDockerCluster class had way too many responsibilities handling attributes corresponding to specific containers
  • There were some redundancies in the test step definitions
  • Custom images are built separately for every test case

Changes made to resolve the issues:

  • The SingleNodeDockerCluster (single node referring to the intent of running the containers on a single compute node) is now defined as a set of Containers with a single Docker network connecting them
  • All test scenarios now have a single cluster which makes the cleanup much simpler and no multiple named clusters in the test steps are needed
  • Introducing the container concept in the code with the Container class from which all used containers are inherited from defining their own attributes, deployment functions and so on. This moves container specific functionality from the SingleNodeDockerCluster and simplifies the deployment of the cluster
  • Removing the redundancies from the step descriptions which simplifies the test scenarios
  • Adding ImageStore to cache built custom images and reuse them in additional testcases, additionally NiFi and MiNiFi configuration files are attached as volumes instead of building them in the image
  • Fixing some timing issues to lower runtimes

Jira ticket: https://issues.apache.org/jira/browse/MINIFICPP-1596
This should also fix the following Jira regarding transient failures in cleanup: https://issues.apache.org/jira/browse/MINIFICPP-1516


Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

  • Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

  • Has your PR been rebased against the latest commit within the target branch (typically main)?

  • Is your initial contribution a single, squashed commit?

For code changes:

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file?
  • If applicable, have you updated the NOTICE file?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

@lordgamez lordgamez marked this pull request as ready for review June 30, 2021 13:29
@lordgamez lordgamez force-pushed the MINIFICPP-1596 branch 3 times, most recently from bd24488 to d9b5b67 Compare August 4, 2021 16:48
@lordgamez lordgamez force-pushed the MINIFICPP-1596 branch 2 times, most recently from e35d6d0 to b8a5931 Compare August 11, 2021 11:25
@lordgamez lordgamez force-pushed the MINIFICPP-1596 branch 2 times, most recently from 16c29d4 to 7382d99 Compare August 25, 2021 13:02
Co-authored-by: Ferenc Gerlits <fgerlits@users.noreply.github.com>
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.

4 participants