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

Make pod logs configurable with config map. #1252

Conversation

AndrienkoAleksandr
Copy link
Contributor

@AndrienkoAleksandr AndrienkoAleksandr commented Apr 26, 2023

Changes

Make pipelines-as-code log level configurable with help of configmap.

Demo

https://youtu.be/pHId9-WT-Ds

Submitter Checklist

  • ♽ Run make test lint before submitting a PR (ie: with pre-commit, no need to waste CPU cycle on CI
  • 📖 If you are adding a user facing feature or make a change of the behavior, please verify that you have documented it
  • 🧪 100% coverage is not a target but most of the time we would rather have a unit test if you make a code change.
  • 🎁 If that's something that is possible to do please ensure to check if we can add a e2e test.
  • 🔎 If there is a flakiness in the CI tests then don't necessary ignore it, better get the flakyness fixed before merging or if that's not possible there is a good reason to bypass it. (token rate limitation may be a good reason to skip).

Copy link
Member

@savitaashture savitaashture left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution

There are some comment I have added

And also could you please add the doc related to how to change configuration of pac-config-logging
ex: https://github.com/tektoncd/triggers/blob/main/docs/troubleshooting.md#configuring-debug-logging-for-eventlisteners

cmd/pipelines-as-code-controller/main.go Outdated Show resolved Hide resolved
cmd/pipelines-as-code-controller/main.go Outdated Show resolved Hide resolved
config/700-config-logging.yaml Outdated Show resolved Hide resolved
Copy link
Member

@chmouel chmouel left a comment

Choose a reason for hiding this comment

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

@savitaashture is it coherent with operator configuration?

@chmouel
Copy link
Member

chmouel commented Apr 26, 2023

/retest

@pipelines-as-code
Copy link

pipelines-as-code bot commented Apr 26, 2023

Golang test coverage difference report

Coverage increased by 2.58%. 🏅 Keep it up 🏅

Package report
package                                                                            before    after    delta
-------                                                                           -------  -------  -------
pkg/acl                                                                           100.00%  100.00%         
pkg/action                                                                         68.75%   68.75%         
pkg/adapter                                                                        72.41%   72.41%         
pkg/apis/features                                                                 100.00%  100.00%         
pkg/cli/info                                                                       88.23%   88.23%         
pkg/cli/prompt                                                                     55.38%   74.46%  +19.08%
pkg/cli/status                                                                     95.23%   95.23%         
pkg/cli/webhook                                                                    49.40%   59.36%   +9.96%
pkg/cmd/tknpac/bootstrap                                                            2.11%    5.72%   +3.61%
pkg/cmd/tknpac/completion                                                          50.00%   50.00%         
pkg/cmd/tknpac/create                                                              44.14%   44.14%         
pkg/cmd/tknpac/describe                                                            46.31%   46.31%         
pkg/cmd/tknpac/generate                                                            62.20%   62.20%         
pkg/cmd/tknpac/info                                                                62.50%   62.50%         
pkg/cmd/tknpac/list                                                                46.47%   46.47%         
pkg/cmd/tknpac/resolve                                                             71.42%   71.42%         
pkg/cmd/tknpac/webhook                                                             52.47%   52.47%         
pkg/consoleui                                                                      82.60%   84.12%   +1.52%
pkg/customparams                                                                        -   92.64%      new
pkg/events                                                                         73.33%   73.33%         
pkg/formatting                                                                     98.71%   98.71%         
pkg/git                                                                            84.84%   84.84%         
pkg/hub                                                                            90.00%   90.62%   +0.62%
pkg/kubeinteraction                                                                52.50%   52.50%         
pkg/kubeinteraction/status                                                         77.27%   77.27%         
pkg/matcher                                                                        86.47%   86.47%         
pkg/params/clients                                                                 14.86%   14.86%         
pkg/params/settings                                                                79.48%   79.48%         
pkg/pipelineascode                                                                 64.95%   80.90%  +15.95%
pkg/provider                                                                       76.19%   76.19%         
pkg/provider/bitbucketcloud                                                        87.09%   86.98%   -0.11%
pkg/provider/bitbucketserver                                                       88.88%   88.18%   -0.70%
pkg/provider/gitea                                                                 34.14%   33.44%   -0.70%
pkg/provider/github                                                                82.05%   83.03%   +0.98%
pkg/provider/github/app                                                            78.33%   78.33%         
pkg/provider/gitlab                                                                86.64%   86.56%   -0.08%
pkg/random                                                                        100.00%  100.00%         
pkg/reconciler                                                                     42.05%   46.25%   +4.20%
pkg/resolve                                                                        87.93%   87.93%         
pkg/secrets                                                                        92.85%   92.85%         
pkg/sort                                                                           51.20%   51.20%         
pkg/sync                                                                           91.13%   91.13%         
pkg/templates                                                                     100.00%  100.00%         
pkg/webhook                                                                        22.22%   22.22%         
                                                                          total:   65.25%   67.83%   +2.58%

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@savitaashture
Copy link
Member

savitaashture commented Apr 28, 2023

@savitaashture is it coherent with operator configuration?

Yes i think its coherent with Operator, Pipeline, Triggers because now with this PR uses config-logging cm for showing logs

But i have a question to @AndrienkoAleksandr that once controller starts and if i edit cm pac-config-logging for configuration i see its not effected

@savitaashture
Copy link
Member

cc @vdemeester

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr AndrienkoAleksandr force-pushed the makePodLogsConfigurableWithConfigMap branch from c98eb95 to 4e40f02 Compare April 30, 2023 13:43
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr
Copy link
Contributor Author

But i have a question to @AndrienkoAleksandr that once controller starts and if i edit cm pac-config-logging for configuration i see its not effected

I provided doc with explanation and I attached demo. I think this stuff should work.

@savitaashture
Copy link
Member

/retest

@chmouel
Copy link
Member

chmouel commented May 2, 2023

you can fix most of those markdownlint error by doing :

make fix-markdownlint

(assuming you have markdownlint binary installed https://github.com/DavidAnson/markdownlint)

/cc @piyush-garg if you want to add this to the devguide if you do a pr update on this

## Configuring debug logging

pipeline-as-code uses ConfigMap named `pac-config-logging` in the same namespace (`pipelines-as-code` by default) with controllers. To get configmap use the command:

Copy link
Contributor

Choose a reason for hiding this comment

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

"the ConfigMap"
the same namespace "as the controllers" instead of "with controllers"
"To get the ConfigMap use the following command:"

pac-config-logging 4 9m44s
```

To retrieve configmap content:
Copy link
Contributor

Choose a reason for hiding this comment

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

"the content of the ConfigMap"

}
```

Controllers log level defined in the `loglevel.*` fields:
Copy link
Contributor

Choose a reason for hiding this comment

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

The loglevel.* fields define the log level for the controllers:

```

Controllers log level defined in the `loglevel.*` fields:
- loglevel.pipelinesascode - log level for pipelines-as-code-controller component
Copy link
Contributor

Choose a reason for hiding this comment

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

"the log level for the ..." - applies to all three lines

- loglevel.pipelines-as-code-webhook - log level for pipelines-as-code-webhook component
- loglevel.pac-watcher - log level for pipelines-as-code-watcher component

You can change log level from `info` to `debug` or any other supported values. For example, set up log level `debug` for pipelines-as-code-watcher component:
Copy link
Contributor

Choose a reason for hiding this comment

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

"change the log level"
"For example, select the debug log level for the...."

```

After that coresponding controller should get a new log level value.
If you want to use the same log level for all pipelines-as-code components, then you have
Copy link
Contributor

Choose a reason for hiding this comment

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

remove "then you have to"

$ kubectl patch configmap pac-config-logging -n pipelines-as-code --type json -p '[{"op": "replace", "path": "/data/loglevel.pac-watcher", "value":"debug"}]'
```

After that coresponding controller should get a new log level value.
Copy link
Contributor

Choose a reason for hiding this comment

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

"After this command, the controller gets a new log level value"

$ kubectl patch configmap pac-config-logging -n pipelines-as-code --type json -p '[ {"op": "remove", "path": "/data/loglevel.pac-watcher"}, {"op": "remove", "path": "/data/loglevel.pipelines-as-code-webhook"}, {"op": "remove", "path": "/data/loglevel.pipelinesascode"}]'
```

In this case pipelines-as-code components should get common log level from `zap-logger-config` - `level` field from the json.
Copy link
Contributor

Choose a reason for hiding this comment

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

"In this case, all pipelines-as-code components get a common log level"


In this case pipelines-as-code components should get common log level from `zap-logger-config` - `level` field from the json.

List zap supported log level values:
Copy link
Contributor

Choose a reason for hiding this comment

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

"zap-logger-config supports the following log levels:"

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
@chmouel
Copy link
Member

chmouel commented May 4, 2023

/retest

@chmouel
Copy link
Member

chmouel commented May 4, 2023

i fixed the markdownliknt for you, we are about to release 0.19 for osp 1.11 i am happy to merge this but we will need a follow up pr addressing @mramendi doc changes to get in for the minor release

(we won't merge feature after tomorrow feature freeze)

@chmouel chmouel merged commit 2cbe1d2 into openshift-pipelines:main May 4, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants