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

Add IO Architecture doc and metrics sample #230

Merged

Conversation

martinkunc
Copy link
Contributor

Adding architecture document and metrics sample

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 20, 2020
docs/arch.md Outdated
to cloud.redhat.com for analysis.

Insights Operator itself is not managing any applications, it rather only runs using Operators Framework infrastructure.
Like is the convention of Operator applications it has most of the code structured in pkg package and pkg/controller/operator.go
Copy link
Contributor

Choose a reason for hiding this comment

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

What about to distinguish the names by using <code> or something like that?

docs/arch.md Outdated
}
]
```
The status is being updated by pkg/controller/status/status.go. Status runs a background task, which is periodically updating
Copy link
Contributor

@tremes tremes Oct 20, 2020

Choose a reason for hiding this comment

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

typo ? Status runs as background task

docs/arch.md Outdated
- endpoint (where to upload to),
- interval (baseline for how often to gather and upload)
- httpProxy, httpsProxy, noProxy eventually to set custom proxy, which overrides cluster proxy just for Insights Operator uploads
- username, password - if set, the insights client upload will be authentized by basic authorization and this username/password. By default it uses Token from pull-secret secret.
Copy link
Contributor

Choose a reason for hiding this comment

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

probably typo authentized

docs/arch.md Outdated
}
```
The configuration secrets are periodically refreshed by [configobserver](pkg/config/configobserver/configobserver.go). Any code can register to
receive signal throught channel by using config.ConfigChanged(), like for example in insightsuploader.go. It will then get notified if config changes.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo throught

docs/arch.md Outdated

### Scheduling and running of Uploader
The operator.go is starting background task defined in pkg/insights/insightsuploader/insightsuploader.go. The insights uploader is periodically checking if there are any data to upload by calling summarizer.
if any exists. If no data to upload are found the uploader continues with next cycle.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfinished sentence? if any exists

docs/arch.md Outdated
### Scheduling and running of Uploader
The operator.go is starting background task defined in pkg/insights/insightsuploader/insightsuploader.go. The insights uploader is periodically checking if there are any data to upload by calling summarizer.
if any exists. If no data to upload are found the uploader continues with next cycle.
The uploader cycle is running wait.Poll function which is waiting until config changes or until there is a time to upload. The time to upload is determined set to initialDelay.
Copy link
Contributor

Choose a reason for hiding this comment

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

The time to upload is determined set to initialDelay.

Reads weird to me... I would suggest "The time is set by initialDelay"

docs/arch.md Outdated
can be either from Proxy settings or from support secret, or from Env variables (cluster-wide).

## Summarising the content before upload
Summarizer is defined by pkg/record/diskrecorder/diskrecorder.go and is merging all existing archives. That means all archives with insights-*.tar.gz pattern which weren't removed and since last check are included. Then mergeReader is taking one file after another and adding all of them to archive under their path.
Copy link
Contributor

Choose a reason for hiding this comment

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

That means all archives with insights-*.tar.gz pattern which weren't removed and since last check are included.

We should try to update this sentence somehow IMO :)

docs/arch.md Outdated
Another background task started from Observer is from pkg/config/configobserver/configobserver.go. The observer creates configObserver by calling configObserver.New, which sets default observing interval to 5 minutes.
The Run method runs again wail.Poll every 5 minutes and reads both support and pull-secret secrets.

## Scheduling diskprunner and what it does
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably diskpruner

docs/arch.md Outdated
Each result is being stored into record.Item as Marshallable item. It is using either golang Json marshaller, or K8s Api serializers. Those has to be explicitly registered in init func. The record is created to archive under its Name specifying full relative path including folders. The extension for particular record file is defined by GetExtension() func, but most of them are today of "json", except metrics or id.

## Downloading and exposing Archive Analysis
After the successfull upload of archive, the progress monitoring task starts. By default it waits for 1m until it checks if results of analysis of the archive (done by external pipeline in cloud.redhat.com) are available. The report contains LastUpdatedAt timestamp, and verifies if report has changed its state (for this cluster) since the last time. If there was no
Copy link
Contributor

Choose a reason for hiding this comment

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

successful

docs/arch.md Outdated
Both these libraries are generated by automation, which specifies from which Api repo and which Api Group it generates it.
All these clients are created in `[operator.go](pkg/controller/operator.go)` from the KUBECONFIG envvar defined in cluster and passed into `[clusterconfig.go](pkg/controller/clusterconfig.go)`.

## The are credentials used in clients
Copy link
Member

@psimovec psimovec Oct 21, 2020

Choose a reason for hiding this comment

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

probably a typo on this line (There are credentials? or something like that)

 Please enter the commit message for your changes. Lines starting
@martinkunc
Copy link
Contributor Author

/test e2e-aws-upgrade


## Scheduling the ConfigObserver
Another background task started from Observer is from `pkg/config/configobserver/configobserver.go`. The observer creates configObserver by calling `configObserver.New`, which sets default observing interval to 5 minutes.
The Run method runs again wail.Poll every 5 minutes and reads both support and pull-secret secrets.
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a typo, I guess it's wait.Poll

```

But adding Bearer token and creating Rest query is all handled automatically for us by using Clients, which are generated, type safe golang libraries,
like `[github.com/openshift/client-go](github.com/openshift/client-go)` or `[github.com/kubernetes/client-go](github.com/kubernetes/client-go)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@Serhii1011010 Serhii1011010 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 12, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: martinkunc, Sergey1011010

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

6 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@tremes
Copy link
Contributor

tremes commented Nov 18, 2020

/test insights-operator-e2e-tests

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@tremes
Copy link
Contributor

tremes commented Nov 18, 2020

/test insights-operator-e2e-tests

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@tremes
Copy link
Contributor

tremes commented Nov 18, 2020

/test e2e

@openshift-merge-robot openshift-merge-robot merged commit 01473c3 into openshift:master Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants