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

poc(metricsforwarder): read configuration from environment instead of filesystem #3125

Merged
merged 47 commits into from
Aug 29, 2024
Merged

poc(metricsforwarder): read configuration from environment instead of filesystem #3125

merged 47 commits into from
Aug 29, 2024

Conversation

bonzofenix
Copy link
Contributor

@bonzofenix bonzofenix commented Aug 9, 2024 •
edited
Loading

Problem:

  • Deploying metricsfowarder needs to regenerate mtar file on each deploy

Solution:

  • Read DB credentials and configuration from user-provided services.
  • Read service configuration from an environment variable.
  • Start metricsforwarder app without relying on the metricsforwarder VM to fetch configuration.

@bonzofenix bonzofenix marked this pull request as draft August 9, 2024 09:25
@bonzofenix bonzofenix force-pushed the metricsforwarder-mtar branch from b209d51 to 9e84db9 Compare August 9, 2024 09:50
@bonzofenix bonzofenix force-pushed the metricsforwarder-mtar branch from 9e84db9 to 7646711 Compare August 9, 2024 10:00
@bonzofenix bonzofenix force-pushed the metricsforwarder-mtar branch from 31454be to 8e3ac4a Compare August 9, 2024 10:44
@bonzofenix bonzofenix changed the title Metricsforwarder mtar feature(metricsforwarder): deploy Metricsforwarder using mtar instead of cf push Aug 9, 2024
@bonzofenix bonzofenix force-pushed the metricsforwarder-mtar branch from ab43a97 to 5c73f35 Compare August 9, 2024 11:12
bonzofenix and others added 6 commits August 9, 2024 13:14
@bonzofenix
Add nosec comment to bypass G101 security warning in metricsforwarder…
75dcb82 
… config test
@bonzofenix
Remove package-lock.json and package.json from autoscaler module
be6e1cc
@bonzofenix
Change deployment directory and use mta-deploy in deploy-apps.sh
ce1d1ef 
 - Update deploy function to change into the autoscaler directory instead of metricsforwarder
 - Replace `make cf-push` with `make mta-deploy` for deploying autoscaler apps
 - Mark `mta-deploy` as .PHONY in Makefile, replacing `cf-push`
@bonzofenix
Merge branch 'main' into metricsforwarder-mtar
7b013e5
@bonzofenix
merge
0b3a3b6
@bonzofenix
Merge branch 'main' into metricsforwarder-mtar
70d33a8
@bonzofenix bonzofenix changed the title feature(metricsforwarder): deploy Metricsforwarder using mtar instead of cf push poc(metricsforwarder): read configuration from environment instead of filesystem Aug 16, 2024
@bonzofenix bonzofenix force-pushed the metricsforwarder-mtar branch from 7180e99 to 463beb4 Compare August 19, 2024 11:49
@bonzofenix
Refactor config loading and add VCAP_SERVICES support in metricsforwa…
e70dbea 
…rder

 • Extract YAML file decoding into DecodeYamlFile function for cleaner LoadConfig.
 • Implement readDbFromVCAP and readConfigFromVCAP to load database and other configurations from VCAP_SERVICES.
 • Update loadVCAPEnvs to use the new VCAP reading functions.
 • Modify tests to handle new environment variable-based configuration loading and add checks for VCAP_SERVICES parsing.
 • Fix error handling in readDbFromVCAP to return an error when multiple DB services are tagged as relational.
@bonzofenix
Add early return for existing DB config in metricsforwarder config re…
9959d71 
…ad function
@bonzofenix bonzofenix force-pushed the metricsforwarder-mtar branch 3 times, most recently from 03e35f9 to cd6bb23 Compare August 20, 2024 15:30
silvestre
silvestre approved these changes Aug 27, 2024
View reviewed changes
src/autoscaler/configutil/cf.go Outdated Show resolved Hide resolved
src/autoscaler/configutil/cf.go Outdated Show resolved Hide resolved
src/acceptance/assets/app/go_app/Makefile Outdated Show resolved Hide resolved
@bonzofenix
Remove unused error variable and update error handling in VCAP config
0e0dbf4 
 • Remove ErrReadEnvironment variable from cf.go
 • Change error handling to print a message instead of returning an error when failing to read VCAP_APPLICATION
 • Add tests for IsRunningOnCF when VCAP_APPLICATION is not set
 • Remove unused unsetEnvVars function and related test scenarios in cmd metricsforwarder_test.go
@bonzofenix
Update VCAPConfiguration to return JSON marshaled credential content
9d5e2a2
@bonzofenix
fix failing tests
9756c37
@bonzofenix
Refactor VCAPConfiguration methods to use service tags instead of names
47f5970
@bonzofenix
Update go-cfenv package and dependencies
3484259 
 - Change import path from cloudfoundry-community to cloud-gov for go-cfenv
 - Upgrade go-cfenv to v1.19.1
 - Update mapstructure package to v1.5.0
@bonzofenix
Remove cf-build target from go_app Makefile
b62e9fb
@bonzofenix
Fix lints
1fad7aa
@bonzofenix bonzofenix added the allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. label Aug 28, 2024
bonzofenix
bonzofenix commented Aug 28, 2024
View reviewed changes
Copy link
Contributor Author

@bonzofenix bonzofenix left a comment

Choose a reason for hiding this comment

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

lgtm

src/autoscaler/configutil/cf.go Show resolved Hide resolved
@bonzofenix bonzofenix force-pushed the metricsforwarder-mtar branch from ef5f37a to cee35f8 Compare August 28, 2024 13:51
@bonzofenix
Copy link
Contributor Author

build with mantic image: https://concourse.app-runtime-interfaces.ci.cloudfoundry.org/teams/app-autoscaler/pipelines/app-autoscaler-release-metricsforwarder-mtar/jobs/acceptance-log-cache-syslog-cf/builds/67

@bonzofenix
Copy link
Contributor Author

working build with noble https://concourse.app-runtime-interfaces.ci.cloudfoundry.org/teams/app-autoscaler/pipelines/app-autoscaler-release-metricsforwarder-mtar/jobs/acceptance-log-cache-syslog-cf/builds/68

@bonzofenix bonzofenix enabled auto-merge August 28, 2024 16:12
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
5 Security Hotspots

See analysis details on SonarCloud

@bonzofenix bonzofenix disabled auto-merge August 28, 2024 20:22
@bonzofenix bonzofenix merged commit 47d35bd into main Aug 29, 2024
89 of 92 checks passed
@bonzofenix bonzofenix deleted the metricsforwarder-mtar branch August 29, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silvestre silvestre silvestre approved these changes

Assignees
No one assigned
Labels
allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. exclude-from-changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bonzofenix @silvestre