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

BUGFIX: ods-deployments file repeats image in mono repository setup #1023

Merged
merged 23 commits into from
Apr 30, 2024
Merged

BUGFIX: ods-deployments file repeats image in mono repository setup #1023

merged 23 commits into from
Apr 30, 2024

Conversation

serverhorror
Copy link
Contributor

@serverhorror serverhorror commented Jul 24, 2023 •
edited by hrcornejo
Loading

This allows us to extract the relevant pod data
in case there are multiple pods in a mono-repo like rollout

fixes #1011

@serverhorror
Adds a check for 'resourceName'
2c9d4eb 
This allows us to extract the relevant pod data
in case there are multiple pods in a mono-repo like rollout

fixes opendevstack#1011
fixes opendevstack#1011
@serverhorror
codeNarc: Fix indentation
05d0b93
@serverhorror
Fix changelog enforcer
22b3571
@serverhorror
Fix build
f76dfba 
This fixes an issue where the build bails out with:

```text
•	Task :codenarcMain FAILED
FAILURE: Build failed with an exception.
•	V/hat went wrong:
Execution failed for task ':codenarcMain'.
•	BUG! UNCAUGHT EXCEPTION: java.lang.invoke.MethodHandles$Lookup.<init>(java.lang.Class,int)
```
@serverhorror
codeNarc: Fix parameterCount warning
86e1d58 
THIS IS AN UNRELATED CHANGE

It fixes:

```
Number of parameters in constructor of class org.ods.orchestration.util.DocumentHistoryEntry exceeds maximum allowed (5).
```
@serverhorror
Add ChangeLog entry for unrelated change
0a034e4
@serverhorror
Fix test for 'run successfully with helm'
6020434
@serverhorror
Fix test for 'run successfully with helm'
547205a
@serverhorror
(chore): Removed MaxPermSize as this had no effect since Java8
3e8a759 
See https://docs.oracle.com/javase/9/migrate/#GUID-1F270BDA-50B0-49C8-807E-0B727CCC5169
@serverhorror
Fix call to checkForPodData in test
f919820 
We are now more graceful in what we accept as valid in this test.
@serverhorror serverhorror marked this pull request as ready for review July 25, 2023 08:00
@serverhorror
Copy link
Contributor Author

@gerardcl: do you have a good possibilty to verify this?

I think it should work but I would need to create a mono-repo like component.

@serverhorror
Copy link
Contributor Author

For verification tests, I recommend to have 2 additional helm components:

  • a simple, single resoruce/pod component
  • a mono-repo like component with multiple pods

serverhorror
serverhorror commented Jul 25, 2023
View reviewed changes
src/org/ods/orchestration/util/DocumentHistoryEntry.groovy Outdated
@@ -11,6 +11,7 @@ class DocumentHistoryEntry implements Map, Serializable {
private final String docVersion
String rational

@SuppressWarnings('ParameterCount')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please not this is unrelated to the actual change, it was reported by codeNarc and I could get it to build without this.

serverhorror
serverhorror commented Jul 25, 2023
View reviewed changes
build.gradle
@@ -137,7 +136,7 @@ test {
}

codenarc {
toolVersion = '1.5'
toolVersion = '1.6'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please note: this is unrelated to the actual changes, it wouldn't let me build without that being set.

serverhorror
serverhorror commented Jul 25, 2023
View reviewed changes
build.gradle
@@ -121,7 +121,6 @@ test {
systemProperty 'com.athaydes.spockframework.report.projectName', 'ods-jenkins-shared-library'
systemProperty 'com.athaydes.spockframework.report.projectVersion', version
maxHeapSize = "2048m"
Copy link
Contributor Author

@serverhorror serverhorror Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the changelog entry, the jvmArgs "-XX:MaxPermSize=256m" option (specifically MaxPermSize) has been deprecated and will prohibit a JVM from starting. It was removed somewhere between 8 and 11, if my research is correct.

@serverhorror
Simplify code
0da017a 
This removes the nested closures. The `obj.every { ... }` was incorrect anyway.
This also allows us to test more easily,
as we are not mixing data retrieval and data munging in the same method any more
@serverhorror
test parsePodJson (called from checkPodData) with multiple pods as input
be3cb51
@serverhorror
(chore) fix formatting
78a2d15
@serverhorror serverhorror changed the title Adds a check for 'resourceName' BUGFIX: ods-deployments file repeats image in mono repository setup Jul 28, 2023
@serverhorror
Copy link
Contributor Author

@jafarre-bi, @jorge-romero, @hrcornejo -- can you review this?

@quenteum quenteum mentioned this pull request Aug 3, 2023
Closed
@serverhorror serverhorror mentioned this pull request Aug 3, 2023
Closed
@serverhorror
Merge branch 'master' into bugfix/ods-deployments-file-repeats-image-…
6b40e51 
…in-mono-repository-setup

# Conflicts:
#	src/org/ods/component/HelmDeploymentStrategy.groovy
@serverhorror
make CodeNarc happ[y|ier]
203db89 
(cherry picked from commit e4a6fe9)
@serverhorror
Merge branch 'master' into bugfix/ods-deployments-file-repeats-image-…
e014da5 
…in-mono-repository-setup

# Conflicts:
#	CHANGELOG.md
#	src/org/ods/component/HelmDeploymentStrategy.groovy
#	src/org/ods/openshift/OpenShiftResourceMetadata.groovy
@serverhorror
Copy link
Contributor Author

@jafarre-bi, @jorge-romero, @hrcornejo -- can you review this?

1 similar comment
@serverhorror
Copy link
Contributor Author

@jafarre-bi, @jorge-romero, @hrcornejo -- can you review this?

@serverhorror
Merge remote-tracking branch 'origin/master' into bugfix/ods-deployme…
06e1e65 
…nts-file-repeats-image-in-mono-repository-setup

# Conflicts:
#	CHANGELOG.md
@jafarre-bi
Copy link
Contributor

Pending testing of the latest changes. Please, @serverhorror, let us know when it's been verified.

@serverhorror
Merge branch 'master' into bugfix/ods-deployments-file-repeats-image-…
bf8eccb 
…in-mono-repository-setup

# Conflicts:
#	CHANGELOG.md
@serverhorror serverhorror mentioned this pull request Apr 9, 2024
Closed
@hrcornejo
Copy link
Contributor

Please @serverhorror resolve the conflicts in CHANGELOG.md

@hrcornejo hrcornejo self-requested a review April 30, 2024 08:26
@hrcornejo hrcornejo merged commit 61da8e2 into opendevstack:master Apr 30, 2024
2 checks passed
BraisVQ pushed a commit that referenced this pull request Jun 6, 2024
@serverhorror @BraisVQ
BUGFIX: ods-deployments file repeats image in mono repository setup (#…
a9975ba 
…1023)

Co-authored-by: Martin Marcher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hrcornejo hrcornejo hrcornejo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ods-deployments file repeats image in mono repository setup
3 participants
@serverhorror @jafarre-bi @hrcornejo