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

refactor: Replace yaml manifests with k8s resources and map implicit dependencies #639

Closed
wants to merge 9 commits into from

Conversation

bryantbiggs
Copy link
Contributor

What does this PR do?

  • Replaces yaml manifest for EKS ADOT plugin with K8s resources - removes need to add kubectl provider to obsv. examples
  • Map implicit dependencies to aid in deployment ordering (cert_manager -> opentelemetry-operator / eks-adot -> collector)
  • Add terraform-aws-managed-service-prometheus; this provisions the AMG workspace but also provides the workspace IAM role for single account usage, workspace SAML configuration, license assocaition, and role association. Once r/grafana_workspace_api_key hashicorp/terraform-provider-aws#25100 is resolved, the extra step of manually fetching API keys will be resolved and handled by code, and aws_grafana_role_association fails to associate AWS SSO user/group IDs hashicorp/terraform-provider-aws#24166 will resolve user management via code as well. This reduces the steps required by users to streamline the provisioning of the example
  • Replace blueprints implementation of prometheus module with terraform-aws-managed-service-prometheus which implements the workspace as well as the alert manager definition and rule group namespaces
  • Refactor opentelemetry-operator to add support for current self-managed addon route while also supporting the new EKS ADOT addon
  • Refactor adot-collector-* addons to flatten configuration to main.tf

⚠️ Note - this is a PR into @bonclay7 's existing PR, not into main

Motivation

  • Improve blueprint usability
  • Ensure support for both self-managed opentelemetry-operator and EKS ADOT addon
  • Better inter-dependency management between addons

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have added a new example under examples to support my PR
  • Yes, I have created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

…dependencies

Venkataraman and others added 9 commits June 7, 2022 10:58
Applies these changes:
- Operator needs to be installed before application to avoid deployment errors
- Cert manager needs to be installed before adot operator
- To avoid "for_each" dependencies issues, cert manager is installed from adot op. module
Base automatically changed from managed-adot to main June 21, 2022 10:26
@bryantbiggs
Copy link
Contributor Author

closing in favor of #664

@bryantbiggs bryantbiggs deleted the fix/self-managed branch June 21, 2022 12:08
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.

2 participants