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.

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

change remote debug tomcat configuration #1276

Merged
merged 3 commits into from
Apr 6, 2023

Conversation

mbussolotto
Copy link
Member

@mbussolotto mbussolotto commented Mar 30, 2023

What does this PR change?

I'm trying to clean up all the tomcat custom configuration in uyuni and sumaform. Currently we have configuration in:

  • /etc/sysconfig/tomcat
  • /etc/tomcat.conf
    and apply them requires parsing the content with some regex.
    This change is aligned with the ones that I'm having for uyuni, where each different configuration is in a different file in /etc/tomcat/conf.d/*conf

Fixes https://github.com/SUSE/spacewalk/issues/19824

@mbussolotto mbussolotto force-pushed the tomcat_debug branch 2 times, most recently from dcd0785 to 3d0d620 Compare March 30, 2023 08:55
@mbussolotto mbussolotto marked this pull request as ready for review March 30, 2023 09:20
Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

Idempotency issue

- name: /etc/sysconfig/tomcat
- pattern: 'JAVA_OPTS="(?!-Xdebug)(.*)"'
/etc/tomcat/conf.d/remote_debug.conf:
file.append:
Copy link
Contributor

Choose a reason for hiding this comment

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

This only works for the first run. You should use a file.replace with append_if_not_found: True instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

it should be better now

@mbussolotto mbussolotto requested a review from cbosdo April 5, 2023 16:11
Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

Using file.touch would be more elegant than managing an empty file... and it avoids resetting the file content if no change is required.

salt/server/tomcat.sls Outdated Show resolved Hide resolved
@mbussolotto mbussolotto requested a review from cbosdo April 6, 2023 08:21
@mbussolotto mbussolotto merged commit ec682b1 into uyuni-project:master Apr 6, 2023
@srbarrios
Copy link
Member

srbarrios commented May 12, 2023

Hi there, just for your information, this PR broke one month ago the pipeline that runs the test suite with a JaCoCo agent in place, to collect test coverage.
I did not had time until now to debug the reasons about what that was failing.

We were injecting the config to add the Java Agent from here, but we were overriding all the values in sysconfig/tomcat file:
https://github.com/SUSE/susemanager-ci/blob/master/terracumber_config/config_files/etc_sysconfig_tomcat

I'm gonna fix it now by adding a new file /etc/tomcat/conf.d/jacoco_agent.conf at our terraform file level, as I need results asap.
(The root issue is that we can't some duplicated options like JWDP, and with our override we had duplicates)

We might want to consider moving JaCoCo enablement for the same place at sumaform level, to avoid the unalignment.

@mbussolotto
Copy link
Member Author

Thanks Oscar, I wasn't aware of that.

But we might want to consider #1305, to avoid the unalignment.

+1

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.

3 participants