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

Bug 1825831: Refactoring collector, add Doc and doc generator #95

Merged

openshift-merge-robot merged 1 commit into openshift:master from martinkunc:refactoring-collector-add-doc

Apr 27, 2020

Contributor

martinkunc commented Apr 17, 2020 •

edited

Loading

The goal here is to

Refactor the clusterconfig.Gather and extract the methods out for better separation and testing
Document the individual methods in code, add description, and location in the archive
Generate documentation from the methods. It collects Gather... methods and if exists complementary Example... methods to collect the anonymized output example.
Resulting documentation is placed in docs/gathered-data.md

openshift-ci-robot requested review from derekwaynecarr and iNecas

April 17, 2020 13:58

Contributor Author

martinkunc commented Apr 17, 2020

/retest

1 similar comment

Contributor Author

martinkunc commented Apr 17, 2020

/retest

Contributor

alexandrevicenzi commented Apr 20, 2020

@martinkunc is there any BZ or Jira related to this PR?

alexandrevicenzi suggested changes

View reviewed changes

Contributor

alexandrevicenzi left a comment

I see some inconsistency in godoc in some function, some include more info some not.

There's a need to document somewhere that samples and everything else are autogen, otherwise, someone might update manually.

pkg/gather/clusterconfig/clusterconfig.go Outdated Show resolved Hide resolved

pkg/gather/clusterconfig/clusterconfig.go Outdated Show resolved Hide resolved

pkg/gather/clusterconfig/clusterconfig.go

-              				if isHealthyOperator(&item) {
+              // GatherMostRecentMetrics gathers cluster Federated Monitoring metrics
+              //
+              // The GET REST query to URL /federate

Contributor

alexandrevicenzi Apr 20, 2020

Sounds a bit better:

Same data as GET /federate

pkg/gather/clusterconfig/clusterconfig.go

+              // The Kubernetes api https://github.com/openshift/client-go/blob/master/config/clientset/versioned/typed/config/v1/clusteroperator.go#L62
+              // Response see https://docs.openshift.com/container-platform/4.3/rest_api/index.html#clusteroperatorlist-v1config-openshift-io
+              //
+              // Location of operators in archive: config/clusteroperator/

Contributor

alexandrevicenzi Apr 20, 2020

One function creates two archives? Should we split this function?

pkg/gather/clusterconfig/clusterconfig.go

+              //
+              // Location of operators in archive: config/clusteroperator/
+              // see: docs/insights-archive-sample/config/clusteroperator
+              // Location of pods in archive: config/pod/

Contributor

alexandrevicenzi Apr 20, 2020

No example for pods?

Contributor Author

martinkunc Apr 21, 2020

I would like to add more samples later. I just have few samples ready and it takes some time to get each prepared with the test.

pkg/gather/clusterconfig/clusterconfig.go

+              // The Kubernetes api https://github.com/kubernetes/client-go/blob/master/kubernetes/typed/core/v1/node.go#L78
+              // Response see https://docs.openshift.com/container-platform/4.3/rest_api/index.html#nodelist-v1core
+              //
+              // Location in archive: config/node/

Contributor

alexandrevicenzi Apr 20, 2020

No See?

docs/gathered-data.md Outdated Show resolved Hide resolved

cmd/gendoc/main.go Show resolved Hide resolved

openshift-ci-robot added the approved label

openshift-ci-robot assigned alexandrevicenzi

martinkunc force-pushed the refactoring-collector-add-doc branch from f8eec5c to ba24d8b Compare

April 21, 2020 08:33

Contributor Author

martinkunc commented Apr 22, 2020

/retest

Contributor Author

martinkunc commented Apr 22, 2020

/retitle Bug 1825831: Refactoring collector, add Doc and doc generator

openshift-ci-robot changed the title ~~Refactoring collector, add Doc and doc generator~~ Bug 1825831: Refactoring collector, add Doc and doc generator

Contributor Author

martinkunc commented Apr 22, 2020

/bugzilla refresh

openshift-ci-robot added the bugzilla/medium label

Contributor

openshift-ci-robot commented Apr 22, 2020

@martinkunc: This pull request references Bugzilla bug 1825831, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the bugzilla/valid-bug label

Contributor

openshift-ci-robot commented Apr 22, 2020

@martinkunc: An error was encountered searching for bug 1825831 on the Bugzilla server at https://bugzilla.redhat.com:

Get https://bugzilla.redhat.com/rest/bug/1825831?api_key=CENSORED: dial tcp 209.132.183.69:443: i/o timeout
Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

In response to this:

Bug 1825831: Refactoring collector, add Doc and doc generator

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Contributor Author

martinkunc commented Apr 22, 2020

/bugzilla refresh

Contributor

openshift-ci-robot commented Apr 22, 2020

@martinkunc: An error was encountered searching for bug 1825831 on the Bugzilla server at https://bugzilla.redhat.com:

Get https://bugzilla.redhat.com/rest/bug/1825831?api_key=CENSORED: dial tcp 209.132.183.69:443: i/o timeout
Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dpensi approved these changes

View reviewed changes

tremes reviewed

View reviewed changes

README.md Outdated Show resolved Hide resolved

tremes reviewed

View reviewed changes

docs/gathered-data.md Outdated Show resolved Hide resolved

tremes approved these changes

View reviewed changes

martinkunc force-pushed the refactoring-collector-add-doc branch from ba24d8b to 1be9355 Compare

April 24, 2020 10:00


          Refactoring collector, add Doc and doc generator

fc6889b

martinkunc force-pushed the refactoring-collector-add-doc branch from 1be9355 to fc6889b Compare

April 24, 2020 10:07

Contributor

alexandrevicenzi commented Apr 27, 2020

/lgtm

openshift-ci-robot added the lgtm label

Contributor

openshift-ci-robot commented Apr 27, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexandrevicenzi, dpensi, martinkunc, tremes

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:

~~OWNERS~~ [alexandrevicenzi,martinkunc]

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

Contributor

openshift-bot commented Apr 27, 2020

/retest

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

Contributor

openshift-bot commented Apr 27, 2020

/retest

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

openshift-merge-robot merged commit f730f2b into openshift:master

Contributor

openshift-ci-robot commented Apr 27, 2020

@martinkunc: All pull requests linked via external trackers have merged: openshift/insights-operator#95. Bugzilla bug 1825831 has been moved to the MODIFIED state.

In response to this:

Bug 1825831: Refactoring collector, add Doc and doc generator

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

This was referenced May 13, 2020

[release-4.3] Bug 1826608: Refactoring collector, add Doc and doc generator #106

Closed

[release-4.3] Bug 1826608: Refactoring collector, add Doc and doc generator #108

Closed

[release-4.2] Bug 1826609: Refactoring collector, add Doc and doc generator #109

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

alexandrevicenzi alexandrevicenzi requested changes

tremes tremes approved these changes

dpensi dpensi approved these changes

derekwaynecarr Awaiting requested review from derekwaynecarr

iNecas Awaiting requested review from iNecas

Labels

approved bugzilla/valid-bug lgtm