Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Cyclic helm tests in test-mda-models #4

Closed
9 tasks done
d-ryan-ashcraft opened this issue Apr 28, 2024 · 4 comments
Closed
9 tasks done

Fix Cyclic helm tests in test-mda-models #4

d-ryan-ashcraft opened this issue Apr 28, 2024 · 4 comments
Assignees
@jaebchoi
Labels
bug Something isn't working
Milestone
1.10.0

Comments

@d-ryan-ashcraft
Copy link
Collaborator

d-ryan-ashcraft commented Apr 28, 2024 •
edited by jaebchoi
Loading

Description

While migrating to the ghcr.io as aissemble's new helm repository, we encountered some cyclic tests in the the test-mda-models module. The core issue is the test-chart execution of maven-exec-plugin, which generally looks like:

            <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>exec-maven-plugin</artifactId>
                <version>3.1.0</version>
                <executions>
                    <execution>
                        <id>test-chart</id>
                        <phase>compile</phase>
                        <goals>
                            <goal>exec</goal>
                        </goals>
                        <configuration>
                            <executable>helm</executable>
                            <arguments>
                                <argument>template</argument>
                                <argument>aissemble-spark-application</argument>
                                <argument>--repo</argument>
                                <argument>${aissemble.helm.repo}</argument>
                                <argument>--version</argument>
                                <argument>${version.aissemble}</argument>
                                <argument>--values</argument>
                                <argument>
                                    src/aissemble_test_data_delivery_pyspark_model_basic/resources/apps/pyspark-data-delivery-basic-base-values.yaml,tests/resources/apps/pyspark-data-delivery-basic-test-values.yaml
                                </argument>
                                <argument>-s</argument>
                                <argument>templates/deployment.yaml</argument>
                            </arguments>
                            <outputFile>target/apps/pyspark-data-delivery-basic-test-chart.yaml</outputFile>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

In this block, we reference a remote helm repo:

                                <argument>--repo</argument>
                                <argument>${aissemble.helm.repo}</argument>

Of course in the process of migrating to a new helm repo, no charts have been deployed here yet, which uncovered the cycle. We should likely do the following:

  • update to use the local version of the chart
  • refactor to at least pull this up to a common pluginDependency definition in test-mda-models so it isn't repeated almost verbatim in four different modules. More ideally, we'd eliminate the use of the maven-exec-plugin for this altogether as it is a generally a pretty good sign we have a workaround in place. Since we've forked the helm-maven-plugin, why not support this with it?

This impacts the following four modules:

  • test-mda-data-delivery-model
  • test-mda-data-delivery-model-basic
  • aissemble-test-data-delivery-pyspark-model
  • aissemble-test-data-delivery-pyspark-model-basic

In the meantime, the test-chart execution has been disabled by pulling up the existing skip-test profile in those four modules into a common profile in test-mda-models and making it active by default. There is a link to this issue on that block as well as each test-chart execution to help folks that might encounter it until it is fixed.

Definition of Done

  • update all four modules to use local version of chart in helm testing
  • Update to use helm-maven-plugin instead of maven-exec-plugin EDIT: helm-maven-plugin doesn't support changing output file name so will fall back to using maven-exec-plugin.
  • refactor to pull test helm logic into common pluginDependency in test-mda-models
  • Clean up all workaround logic to skip helm testing.
  • Verify mvn clean install is successful
  • Verify alll four modules can run helm testing upon mvn clean install

Testing Instruction

  • pull in latest build of aissemble
  • Verify mvn clean install is successful
  • Verify alll four modules runs testing successfully upon mvn clean install
  • test-mda-data-delivery-model
  • test-mda-data-delivery-model-basic
  • aissemble-test-data-delivery-pyspark-model
  • aissemble-test-data-delivery-pyspark-model-basic
    EX) Cucumber test ran in test-mda-data-delivery-model-basic is triggered and passed

Screenshot 2024-10-24 at 3 47 19 PM
Screenshot 2024-10-24 at 3 48 01 PM
@d-ryan-ashcraft d-ryan-ashcraft added the bug Something isn't working label Apr 28, 2024
@d-ryan-ashcraft d-ryan-ashcraft added this to the 1.7.0 milestone Apr 28, 2024
aaron-gary added a commit that referenced this issue May 1, 2024
@aaron-gary
# This is a combination of 4 commits.
3e80609 
# This is the 1st commit message:

#2 Add Maven build workflow

# This is the commit message #2:

#2 update build workflow

# This is the commit message #3:

#2 add branch checkout to build workflow

# This is the commit message #4:

#2 Add on event to build workflow
aaron-gary added a commit that referenced this issue May 1, 2024
@aaron-gary
# This is a combination of 4 commits.
9c23a2c 
# This is the 1st commit message:

#2 Add Maven build workflow

# This is the commit message #2:

#2 update build workflow

# This is the commit message #3:

#2 add branch checkout to build workflow

# This is the commit message #4:

#2 Add on event to build workflow
aaron-gary added a commit that referenced this issue May 2, 2024
@aaron-gary
# This is a combination of 31 commits.
3f6ea16 
# This is the 1st commit message:

# This is a combination of 4 commits.
# This is the 1st commit message:

#2 Add Maven build workflow

# This is the commit message #2:

#2 update build workflow

# This is the commit message #3:

#2 add branch checkout to build workflow

# This is the commit message #4:

#2 Add on event to build workflow

# This is the commit message #2:

#2 Remove unused executions

# This is the commit message #3:

#2 Focus on build

# This is the commit message #4:

#2 Remove build execution where not needed

# This is the commit message #5:

#2 debug failing module

# This is the commit message #6:

#2 Remove unused target folder copy

# This is the commit message #7:

#2 Build spark and jenkins docker images

# This is the commit message #8:

#2 Retry full build

# This is the commit message #9:

#2 Omitting module

# This is the commit message #10:

#2 Fix for out of disk space

# This is the commit message #11:

#2 tagging docker images

# This is the commit message #12:

#2 Remove Temporarily remove Docker module

# This is the commit message #13:

#2 Build update

# This is the commit message #14:

#2 Build update

# This is the commit message #15:

#2 move chart dry-runs to IT profile

# This is the commit message #16:

#2 curl delta-hive assembly in docker build

# This is the commit message #17:

#2 cache m2 repo

# This is the commit message #18:

#2 prune docker build cache between images to save space

# This is the commit message #19:

#2 add maven build-cache to GH cache

# This is the commit message #20:

#2 run clean goal in build to clear docker cache

# This is the commit message #21:

#2 set maven caches to always save even if the build failed

# This is the commit message #22:

#2 adjust number of docker modules built

# This is the commit message #23:

#2 use the same cache for .m2 even if poms change

# This is the commit message #24:

#2 change from `save-always` flag to `if: always()` see actions/cache#1315

# This is the commit message #25:

#2 further reduce docker images being built

# This is the commit message #26:

#2 disable modules that depend on helm charts

# This is the commit message #27:

#2 use maven wrapper

# This is the commit message #28:

#2 restore modules to test build-cache

# This is the commit message #29:

#2 fix build of modules with intra-project chart dependencies

# This is the commit message #30:

#2 use explict .m2 repo cache so we can fall-back to older caches

# This is the commit message #31:

#2 save maven caches on build failure
aaron-gary added a commit that referenced this issue May 2, 2024
@aaron-gary
# This is a combination of 31 commits.
bc94735 
# This is the 1st commit message:

# This is a combination of 4 commits.
# This is the 1st commit message:

#2 Add Maven build workflow

# This is the commit message #2:

#2 update build workflow

# This is the commit message #3:

#2 add branch checkout to build workflow

# This is the commit message #4:

#2 Add on event to build workflow

# This is the commit message #2:

#2 Remove unused executions

# This is the commit message #3:

#2 Focus on build

# This is the commit message #4:

#2 Remove build execution where not needed

# This is the commit message #5:

#2 debug failing module

# This is the commit message #6:

#2 Remove unused target folder copy

# This is the commit message #7:

#2 Build spark and jenkins docker images

# This is the commit message #8:

#2 Retry full build

# This is the commit message #9:

#2 Omitting module

# This is the commit message #10:

#2 Fix for out of disk space

# This is the commit message #11:

#2 tagging docker images

# This is the commit message #12:

#2 Remove Temporarily remove Docker module

# This is the commit message #13:

#2 Build update

# This is the commit message #14:

#2 Build update

# This is the commit message #15:

#2 move chart dry-runs to IT profile

# This is the commit message #16:

#2 curl delta-hive assembly in docker build

# This is the commit message #17:

#2 cache m2 repo

# This is the commit message #18:

#2 prune docker build cache between images to save space

# This is the commit message #19:

#2 add maven build-cache to GH cache

# This is the commit message #20:

#2 run clean goal in build to clear docker cache

# This is the commit message #21:

#2 set maven caches to always save even if the build failed

# This is the commit message #22:

#2 adjust number of docker modules built

# This is the commit message #23:

#2 use the same cache for .m2 even if poms change

# This is the commit message #24:

#2 change from `save-always` flag to `if: always()` see actions/cache#1315

# This is the commit message #25:

#2 further reduce docker images being built

# This is the commit message #26:

#2 disable modules that depend on helm charts

# This is the commit message #27:

#2 use maven wrapper

# This is the commit message #28:

#2 restore modules to test build-cache

# This is the commit message #29:

#2 fix build of modules with intra-project chart dependencies

# This is the commit message #30:

#2 use explict .m2 repo cache so we can fall-back to older caches

# This is the commit message #31:

#2 save maven caches on build failure
@cpointe-ibllanos
Copy link
Contributor

I can assign this to myself once I am added to the org.

aaron-gary added a commit that referenced this issue May 28, 2024
@aaron-gary
# This is a combination of 9 commits.
ef88aab 
# This is the 1st commit message:

#2 build script for runner

# This is the commit message #2:

#2 add architecture to action workflow

# This is the commit message #3:

#2 Update python version

# This is the commit message #4:

#2 remove python install from workflow

# This is the commit message #5:

#2 update Docker install

# This is the commit message #6:

#2 update docker install

# This is the commit message #7:

#2 update remove docker install from workflow script

# This is the commit message #8:

#2 update remove helm install

# This is the commit message #9:

#2 update get docker info
@ewilkins-csi ewilkins-csi modified the milestones: 1.7.0, 1.8.0 Jun 12, 2024
@csun-cpointe csun-cpointe modified the milestones: 1.8.0, 1.9.0 Aug 5, 2024
@ewilkins-csi ewilkins-csi modified the milestones: 1.9.0, 1.10.0 Sep 25, 2024
@jaebchoi jaebchoi self-assigned this Oct 10, 2024
@jaebchoi jaebchoi added this to the 1.10.0 milestone Oct 10, 2024
@jaebchoi jaebchoi changed the title Cyclic helm tests in test-mda-models Fix Cyclic helm tests in test-mda-models Oct 10, 2024
@jaebchoi
Copy link
Contributor

DoD Completed with @meliz19

jaebchoi added a commit that referenced this issue Oct 23, 2024
@jaebchoi
#4 Fix Cyclic helm tests in test-mda-models
e75e405
jaebchoi added a commit that referenced this issue Oct 23, 2024
@jaebchoi
#4 Fix Cyclic helm tests in test-mda-models
640f18a
jaebchoi added a commit that referenced this issue Oct 23, 2024
@jaebchoi
#4 Fix Cyclic helm tests in test-mda-models
4d53520
@jaebchoi jaebchoi mentioned this issue Oct 23, 2024
Closed
@jaebchoi
Copy link
Contributor

OTS completed with @cwoods-cpointe

jaebchoi added a commit that referenced this issue Oct 24, 2024
@jaebchoi
#4 Fix Cyclic helm tests in test-mda-models
80e1054
jaebchoi added a commit that referenced this issue Oct 24, 2024
@jaebchoi
#4 Fix Cyclic helm tests in test-mda-models
f8e34e8
jaebchoi added a commit that referenced this issue Oct 24, 2024
@jaebchoi
#4 Fix Cyclic test in test mda models
00e8256
@jaebchoi jaebchoi mentioned this issue Oct 24, 2024
Merged
jaebchoi added a commit that referenced this issue Oct 24, 2024
@jaebchoi
#4 Fix Cyclic test in test mda models
06c76d7
jaebchoi added a commit that referenced this issue Oct 24, 2024
@jaebchoi
#4 Fix Cyclic test in test mda models
5a80c62
jaebchoi added a commit that referenced this issue Oct 24, 2024
@jaebchoi
Merge pull request #438 from boozallen/4-cyclic-helm-test
adbb1ce 
#4.  Fix Cyclic test in test mda models
ewilkins-csi pushed a commit that referenced this issue Oct 24, 2024
@jaebchoi @ewilkins-csi
[#4] switch from docker compose to testcontainers
710f5fc 
Our pyspark test metamodel module was using docker-compose to start
containers for integration tests instead of our usual approach of using
testcontainers. This caused a failure in CI as the GitHub runner doesn't
appear to have docker-compose. This changeset also includes some fixes
to the other places we were using the testcontainers Python package.
Namely:
 - We now use dynamic ports in extensions-encryption-python
 - SafeDockerContainer workaround was removed as the core problem was
   resolved in testcontainers 4.0
ewilkins-csi added a commit that referenced this issue Oct 25, 2024
@ewilkins-csi
Merge pull request #439 from boozallen/fixdocker-compose
8ba243d 
[#4] switch from docker compose to testcontainers
@meliz19
Copy link
Contributor

meliz19 commented Oct 25, 2024

Final test completed!

@meliz19 meliz19 closed this as completed Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jaebchoi jaebchoi

Labels
bug Something isn't working
Projects
None yet
Milestone
1.10.0
Development

No branches or pull requests
6 participants
@ewilkins-csi @meliz19 @d-ryan-ashcraft @csun-cpointe @cpointe-ibllanos @jaebchoi