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

[SLOs][Fleet] Install integration SLO assets #186974

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Jun 26, 2024

Summary

Install integration SLO assets !!

How to test with Ngnix Integrations PR

The following instructions describe how to run kibana from source code while leveraging elastic-stack capabilities: elasticsearch, fleet-server, elastic-agent using elastic-package. On a high level, we need to a) build and start the stack from the integrations branch that adds the SLO to the Nginx package, b) stop the Kibana container, c) do some changes to the kibana.dev.yml and then d) start the kibana process from source code.

  1. Fork integrations repo
  2. clone the forked repo locally git clone git@github.com:YOUR_USERNAME/integrations.git
  3. Check out this PR
  4. Move inside the ngnix repo cd packages/ngnix
  5. Build the ngnix package using go run github.com/elastic/elastic-package check
  6. Start the local stack by running go run github.com/elastic/elastic-package stack up -d -v --version 8.15.0-SNAPSHOT
  7. Stop the Kibana container docker stop elastic-package-stack-kibana-1
  8. Go to Kibana repo locally
  9. Update the kibana.dev.yml file as shown below
  10. Setup extra CA certificates for node export NODE_EXTRA_CA_CERTS="$HOME/.elastic-package/profiles/default/certs/kibana/ca-cert.pem"
  11. Run yarn kbn bootstrap
  12. Start kibana yarn start --no-base-path
  13. From Kibana UI go to Integrations App and install the Nginx package

config/kibana.dev.yml

elastic-package creates a local stack that uses HTTPS using generated CA, certificates, and keys. All the certificates are stored locally in the $HOME/.elastic-package/profiles/default/certs/ sub folders. You need to update your kibana.dev.yml file accordingly and replace $HOME to actual path.

server.name: kibana
server.host: "0.0.0.0"
server.ssl.enabled: true
server.ssl.certificate: "$HOME/.elastic-package/profiles/default/certs/kibana/cert.pem"
server.ssl.key: "$HOME/.elastic-package/profiles/default/certs/kibana/key.pem"
server.ssl.certificateAuthorities: ["$HOME/.elastic-package/profiles/default/certs/kibana/ca-cert.pem"]

elasticsearch.hosts: [ "https://127.0.0.1:9200" ]
elasticsearch.ssl.certificateAuthorities: "$HOME/.elastic-package/profiles/default/certs/kibana/ca-cert.pem"

elasticsearch.serviceAccountToken: "<TOKEN>"

monitoring.ui.container.elasticsearch.enabled: true

# xpack.fleet.agents.elasticsearch.hosts: ["https://127.0.0.1:9200"]

xpack.fleet.registryUrl: "https://127.0.0.1:8080"
xpack.fleet.agents.enabled: true
xpack.fleet.agents.fleet_server.hosts: ["https://127.0.0.1:8220"]

xpack.encryptedSavedObjects.encryptionKey: "12345678901234567890123456789012"

xpack.cloudSecurityPosture.enabled: true

xpack.fleet.packages:
  - name: system
    version: latest
  - name: elastic_agent
    version: latest
  - name: fleet_server
    version: latest
xpack.fleet.agentPolicies:
  - name: Elastic-Agent (elastic-package)
    id: elastic-agent-managed-ep
    is_default: true
    is_managed: false
    namespace: default
    monitoring_enabled:
      - logs
      - metrics
    package_policies:
      - name: system-1
        id: default-system
        package:
          name: system
  - name: Fleet Server (elastic-package)
    id: fleet-server-policy
    is_default_fleet_server: true
    is_managed: false
    namespace: default
    package_policies:
      - name: fleet_server-1
        id: default-fleet-server
        package:
          name: fleet_server
xpack.fleet.outputs:
  - id: fleet-default-output
    name: default
    type: elasticsearch
    hosts: [ https://127.0.0.1:9200 ]
    # ca_trusted_fingerprint: "${ELASTIC_PACKAGE_CA_TRUSTED_FINGERPRINT}"
    is_default: true
    is_default_monitoring: true

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

const sloAssets = assetsToInstall.filter((asset) => asset.type === KibanaSavedObjectType.slo);

await installSLOAssets({ sloAssets, sloClient, soClient, esClient, spaceId });

return installedAssets;
Copy link
Contributor

Choose a reason for hiding this comment

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

@shahzad31 Don't we want to return here all installed assets including the sloAssets?

@mgiota
Copy link
Contributor

mgiota commented Jun 28, 2024

@shahzad31 I noticed this

Screenshot 2024-06-28 at 01 20 35

You need to add in this file following:

  slo: i18n.translate('xpack.fleet.epm.assetTitles.slo', {
    defaultMessage: 'SLOs',
  })

UPDATE
It looks like you fixed this already in this commit. It is hard to follow because of the many files included in this PR. Once you clean up the git history, I'll pull again your changes.

@mgiota
Copy link
Contributor

mgiota commented Jun 28, 2024

A couple more things:

When I uninstall Nginx, the SLO doesn't get deleted. I guess you haven't handled this yet, right? We need to make sure that when we delete the integration, the SLO gets deleted

@mgiota
Copy link
Contributor

mgiota commented Jun 28, 2024

We need to investigate further why we get these errors:

Screenshot 2024-06-27 at 23 58 35 Screenshot 2024-06-27 at 23 58 52

const sloAssets = [
...(allAssets ?? []),
{
attributes: {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can try to remove this hardcoded SLO and integrate it with this PR. Not sure how at the moment. Need to look into this. @muthu-mps Any idea how to do it?

Copy link

@muthu-mps muthu-mps Jun 28, 2024

Choose a reason for hiding this comment

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

@mgiota - We can try getting the content using the getAssetFromAssetsMap and remove the hardcoded values.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok cool! @shahzad31 If you can clean up this branch to not contain all these changes from main, then we can do the change recommended by @muthu-mps

@mgiota mgiota changed the title [SLOs] Install integration SLO assets [SLOs][Fleet] Install integration SLO assets Jun 28, 2024

const installedAssets = await installKibanaSavedObjects({
logger,
savedObjectsImporter,
kibanaAssets: assetsToInstall,
kibanaAssets: assetsToInstall.filter((asset) => asset.type !== KibanaSavedObjectType.slo),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if it makes sense to add slo to KibanaSavedObjectType and KibanaAssetType if it's not installed the same way as other kibana assets. Other types are filtered out in line 125.

SLOs could be installed in a separate function added here, where each type of asset has its own function (kibana assets, ilm policies, etc.)

If we still think semantically SLOs belong in kibana assets, I think it makes sense to keep the installation in installKibanaAssets.

Copy link
Member

Choose a reason for hiding this comment

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

+1 if they are installed differently it may be interesting to have his own assets type and persist it in the installation saved object as his own properties.

Having them mixed mean we will have to do some filtering in a lot of codepath deletion/upgrade and it will be easy to miss one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SLOs have saved objects but it also install few other resources like ingest pipelines and transorm, so can't be just bundled under traditional kibana saved objects.

Copy link
Member

Choose a reason for hiding this comment

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

One do you think of introducing a new slo_assets key (by space if SLO are space aware) for the saved object installation? instead of using the kibana_assets one

Copy link
Member

Choose a reason for hiding this comment

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

Also if we go that way it will be great to have a new step installSloAssets so that code is not mixed in the installKibanaAssets part

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes SLOs are space aware.

@nchaulet
Copy link
Member

Hi @shahzad31 we are in the process of making Fleet more space aware, I am curious does SLOs are space aware? in that case we may want to think of a way to install them in multiple space. Similar to what we do with kibana assets #186620

@shahzad31 shahzad31 marked this pull request as ready for review July 16, 2024 16:18
@shahzad31 shahzad31 requested review from a team as code owners July 16, 2024 16:18
@shahzad31 shahzad31 added the release_note:skip Skip the PR/issue when compiling release notes label Jul 16, 2024
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-management Observability Management User Experience Team labels Jul 16, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@mgiota
Copy link
Contributor

mgiota commented Jul 30, 2024

@jsoriano Just want to let you know that this PR is under review. Once we solve some permission issues we found, I guess you need to release a new version of package registry? We will keep you posted, once this PR is merged. Please let us know if there is anything else that needs to be done on our side

@mgiota
Copy link
Contributor

mgiota commented Jul 30, 2024

Added Kibana_admin along with the Kibana system user role. The same error happened.
Then finally added the super_user role. The error is gone when installing the integration with super_user role.

@muthu-mps Thanks! With above mentioned settings, indeed I get no transform error.

@mgiota
Copy link
Contributor

mgiota commented Jul 30, 2024

@elasticmachine merge upstream

@jsoriano
Copy link
Member

@jsoriano Just want to let you know that this PR is under review. Once we solve some permission issues we found, I guess you need to release a new version of package registry? We will keep you posted, once this PR is merged. Please let us know if there is anything else that needs to be done on our side

Thanks for the heads-up!

There are no related changes planed in the package registry for SLOs, so I don't think we need to prepare a release for it. Or is there something missing in package registry responses?

@ishleenk17
Copy link

ishleenk17 commented Jul 31, 2024

  1. Added only kibana_system role to the user. By adding this role I am able to see the transform error happening in the SLO page.

@muthu-mps : Is this a make shift arrangement until the changes are merged into Kibana or is the customer expected to create this user post the merge as well ?

@muthu-mps
Copy link

  1. Added only kibana_system role to the user. By adding this role I am able to see the transform error happening in the SLO page.

@muthu-mps : Is this a make shift arrangement until the changes are merged into Kibana or is the customer expected to create this user post the merge as well ?

The current shift arrangement is to test it locally. We are not going to suggest this change for the customer. Validating what is the suitable way in which we can handle this behaviour.

@grabowskit
Copy link

Based on our weekly call today, we agreed to put this ticket on hold and look into alternative options. The main idea of making SLOs available out of the box with Integrations is not accomplished through this ticket because of the extra work around permissions, Transforms, and license checks. It appears to have the opposite effect of making it easier for customers to onboard SLOs. Further discussion needs to be done on how we can create a template and how SLO and Fleet integration permissions can be incorporated into a better user experience of onboarding SLOs

@ishleenk17
Copy link

Based on our weekly call today, we agreed to put this ticket on hold and look into alternative options. The main idea of making SLOs available out of the box with Integrations is not accomplished through this ticket because of the extra work around permissions, Transforms, and license checks. It appears to have the opposite effect of making it easier for customers to onboard SLOs. Further discussion needs to be done on how we can create a template and how SLO and Fleet integration permissions can be incorporated into a better user experience of onboarding SLOs

Hello @grabowskit,

This is the meta which captures all the things we require to make the SLO's work for Integration Packages.
Discussing and fixing how licensing works is also part of this, regarding which I started discussion in the slack channel here.

IMHO, we should carry on with this change wrt. adding the ability in Fleet to add/delete/update the SLO and carry forward the work required for licenses as part of the other issue.
We may also add any other requirement to the meta ticket. WDYT?

@mgiota
Copy link
Contributor

mgiota commented Aug 1, 2024

@ishleenk17 Thanks for the meta ticket. In the list I would add this issue as well, which handles the permissions errors I found in the current PR.

@muthu-mps : Is this a make shift arrangement until the changes are merged into Kibana or is the customer expected to create this user post the merge as well ?

I made a first attempt to solve the permission issues in this commit and here's how the reauthorization workflow would look like. I haven't pushed the changes in the current PR. I first need to clarify a few things with Fleet team, since I am adapting current Fleet logic to meet our SLO needs.

Screen.Recording.2024-08-01.at.16.40.39.mov

I have a couple more things I would like to clarify with Fleet/Integrations team.

  • What should happen when user deletes the automatically generated SLO from the SLO page? Currently we simply delete the SLO, but we don't do anything special in terms of the installed integration. How should we handle this? Is there a recommended Fleet way to handle managed resources?
  • We currently don't have a visual indicator in our SLO list page to indicate the SLOs that were automatically installed through Integrations
  • License issue you mentioned
  • Anything else we haven't considered?

There are quite a few open questions, that's why further discussions need to be done. As @grabowskit mentioned we put this ticket on hold. Once I am back from PTO in two weeks, we can revisit how we want to move forward. I wouldn't merge current PR as is. Permission issue needs to be solved and pushed into current PR.

@muthu-mps
Copy link

@mgiota - I have created an issue to discuss and finalise on handling the delete use-case.

@muthu-mps
Copy link

While we click the installed SLO from the integration assets tab this opens the SLO in the edit mode. Ideally this should open the SLO in view mode not in the edit mode.

cc: @ishleenk17

@mgiota
Copy link
Contributor

mgiota commented Aug 22, 2024

I am right now reviewing this PR. Once above PR is merged, I would like to re-check current PR. I suspect that privilege issue might be solved with the use of es-secondary-authorization header that is introduced in the above mentioned PR. We might not need to do anymore what I was experimenting with in this commit. Or it might be I need to combine the solutions. Need to further check it again and putting this here as a reminder.

@muthu-mps
Copy link

@shahzad31 -

  • As this PR has been on hold for sometime to address the authorization issue. The authorization issue is now addressed as part of this PR using es-secondary-authorization.
  • Can we move forward as the issue has been resolved?

@lalit-satapathy
Copy link

In order to proceed on this PR, I suggest this PR be merged post checking of https://github.com/elastic/obs-integration-team/issues/50?

@juliaElastic can you please help with codeowner approval?

I suggest the future enhancements can be taken separately:

Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

LGTM

@ishleenk17
Copy link

@shahzad31 : Can we please resolve the conflicts and merge the PR. Looks like we have all the approvals now.

@pzl: Could you please provide code owner spproval.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 10, 2024

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #4 / CreateSLO happy path calls the expected services
  • [job] [logs] Jest Tests #4 / CreateSLO happy path calls the expected services
  • [job] [logs] Jest Tests #4 / CreateSLO happy path overrides the default values when provided
  • [job] [logs] Jest Tests #4 / CreateSLO happy path overrides the default values when provided
  • [job] [logs] Jest Tests #4 / CreateSLO happy path overrides the settings when provided
  • [job] [logs] Jest Tests #4 / CreateSLO happy path overrides the settings when provided
  • [job] [logs] Jest Tests #4 / CreateSLO unhappy path rollbacks completed operations when create temporary document fails
  • [job] [logs] Jest Tests #4 / CreateSLO unhappy path rollbacks completed operations when create temporary document fails
  • [job] [logs] Jest Tests #4 / CreateSLO unhappy path rollbacks completed operations when rollup transform install fails
  • [job] [logs] Jest Tests #4 / CreateSLO unhappy path rollbacks completed operations when rollup transform install fails
  • [job] [logs] Jest Tests #4 / CreateSLO unhappy path rollbacks completed operations when summary transform start fails
  • [job] [logs] Jest Tests #4 / CreateSLO unhappy path rollbacks completed operations when summary transform start fails
  • [job] [logs] FTR Configs #5 / SLO API Tests Reset SLOs updates the SO and transforms
  • [job] [logs] FTR Configs #5 / SLO API Tests Reset SLOs updates the SO and transforms

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 1239 1240 +1
slo 59 84 +25
total +26

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
slo 1 3 +2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 169.8KB 169.9KB +105.0B
Unknown metric groups

API count

id before after diff
fleet 1362 1363 +1
slo 59 84 +25
total +26

ESLint disabled in files

id before after diff
slo 4 3 -1

References to deprecated APIs

id before after diff
slo 3 4 +1

Total ESLint disabled count

id before after diff
slo 18 17 -1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@muthu-mps
Copy link

@shahzad31 - Just pulled the latest code changes to validate few scenarios. The SLO installation was not successful with the recent changes. In the assets tab the SLO ID appears, but I am unable to see the SLO getting created or viewed in the SLO page.
Screenshot 2024-09-11 at 11 32 06 AM

Can you please check it once?

@jsoriano
Copy link
Member

jsoriano commented Oct 9, 2024

@shahzad31 @mgiota do you know if there are plans to ship this feature in 8.16?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-management Observability Management User Experience Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.