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.

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

cmd/image-builder: add deployment channel as field to the logs #1299

Merged
merged 3 commits into from
Aug 16, 2024
Merged

cmd/image-builder: add deployment channel as field to the logs #1299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
917dcd3
cmd/image-builder: add deployment channel as field to the logs
schuellerf Aug 14, 2024
9629f81
test: check the status of the server on startup
schuellerf Aug 14, 2024
542097a
github/workflows: use make to get more consistent
schuellerf Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,8 @@ jobs:
version: v1.56.2
skip-cache: true

- name: install tern
run: go install github.com/jackc/tern@latest

- name: Run unit tests
run: go test -v -race -covermode=atomic -coverprofile=coverage.txt -coverpkg=./... ./...
run: make unit-tests
Copy link
Contributor

Choose a reason for hiding this comment

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

The makefile has a comment re-implementing .github/workflows/tests.yml as close as possible which probably needs updating now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - good catch 🙈


# disabled as 'pinging codecov' hangs
# see https://github.com/codecov/feedback/issues/354
Expand Down
5 changes: 5 additions & 0 deletions cmd/image-builder/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
"github.com/labstack/gommon/log"
slogger "github.com/osbuild/osbuild-composer/pkg/splunk_logger"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -73,6 +74,10 @@ func main() {
}
}

if conf.DeploymentChannel != "" {
logrus.AddHook(&slogger.EnvironmentHook{Channel: conf.DeploymentChannel})
}

if conf.SplunkHost != "" {
err = logger.AddSplunkHook(logrus.StandardLogger(), conf.SplunkHost, conf.SplunkPort, conf.SplunkToken)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ require (
github.com/oapi-codegen/runtime v1.1.1
github.com/osbuild/community-gateway/oidc-authorizer v0.0.0-20240117171535-401ddadefd40
github.com/osbuild/images v0.73.0
github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240311100454-57ebfb401131
github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240814102216-0239db53236d
github.com/prometheus/client_golang v1.19.1
github.com/redhatinsights/app-common-go v1.6.8
github.com/redhatinsights/identity v0.0.0-20220719174832-36a7b1cbeff1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ github.com/osbuild/community-gateway/oidc-authorizer v0.0.0-20240117171535-401dd
github.com/osbuild/community-gateway/oidc-authorizer v0.0.0-20240117171535-401ddadefd40/go.mod h1:CecYUdLQNeKEkEC0DOejBTaVxs8Xbr30tRtgOIpbuKo=
github.com/osbuild/images v0.73.0 h1:2mKFohG1mGh92KFr/rtkmOYTe1Oi6YT0Qdgdz5I20hE=
github.com/osbuild/images v0.73.0/go.mod h1:5xDxWmKDseAPwsnHS16AsHDM2ytCDJcsPK2uaKfvGvc=
github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240311100454-57ebfb401131 h1:T0XABvVgYzQ4MUfJG+WF+2+/lDHXv8ZMRf9Sqeq+N24=
github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240311100454-57ebfb401131/go.mod h1:z+WA+dX6qMwc7fqY5jCzESDIlg4WR2sBQezxsoXv9Ik=
github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240814102216-0239db53236d h1:r9BFPDv0uuA9k1947Jybcxs36c/pTywWS1gjeizvtcQ=
github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240814102216-0239db53236d/go.mod h1:zR1iu/hOuf+OQNJlk70tju9IqzzM4ycq0ectkFBm94U=
github.com/perimeterx/marshmallow v1.1.5 h1:a2LALqQ1BlHM8PZblsDdidgv1mWi1DgC2UmX50IvK2s=
github.com/perimeterx/marshmallow v1.1.5/go.mod h1:dsXbUu8CRzfYP5a87xpp0xq9S3u0Vchtcl8we9tYaXw=
github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4=
Expand Down
1 change: 1 addition & 0 deletions internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type ImageBuilderConfig struct {
RecommendCA string `env:"RECOMMENDATIONS_CA_PATH"`
GlitchTipDSN string `env:"GLITCHTIP_DSN"`
FedoraAuth bool `env:"FEDORA_AUTH"`
DeploymentChannel string `env:"CHANNEL"`
}

func (ibc *ImageBuilderConfig) IsDebug() bool {
Expand Down
5 changes: 4 additions & 1 deletion internal/v1/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,10 @@ func startServer(t *testing.T, tscc *testServerClientsConf, conf *ServerConfig)
require.NoError(t, err)
// execute in parallel b/c .Run() will block execution
go func() {
_ = echoServer.Start("localhost:8086")
err = echoServer.Start("localhost:8086")
if err.Error() != "http: Server closed" {
Copy link
Contributor

Choose a reason for hiding this comment

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

(niptick) I personally would prefer if this commit could be it's own PR (alternatively the PR title could get updated maybe?). When going over the project history it's often nice to see the what PR a commit belonged to and here the PR title/intention and this particular fix feel a bit disconnected.

About the change itself:

It's probably best to compare error types instead of strings here, i.e. if err != http.ErrServerClosed

How does this work exactly? Will we always get ErrServerClosed here or is it random (either nil or ErrServerClosed?).

Bonus points for: the comment above talks about .Run() but the code uses .Start() so we could clean this up too. And while add it we could use your nice GetFreePort() helper from osbuild/osbuild-composer@55c5602 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ServerClose always happens on regular shutdown...
I already did implement with GetFreePort() (locally) but stopped after changing ~1000 strings to take a dynamic host&port 😑
Will do this "major rework" at some other point in time

Copy link
Contributor

@mvo5 mvo5 Aug 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I did a quick sed experiement and opened #1303 which may help, I did not really check much, it was mostly sed doing it's thing but it might be useful. No getFreePort there but that should be easy to hook up once the other pieces are there.

It also include the tiny http.ErrServerClosed tweak in #1304

require.NoError(t, err)
}
}()

// wait until server is ready
Expand Down
2 changes: 2 additions & 0 deletions templates/image-builder.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ objects:
key: proxy
name: recommendations-secrets
optional: true
- name: CHANNEL
value: ${CHANNEL}
volumeMounts:
- name: config-volume
mountPath: /app/config
schuellerf marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
26 changes: 26 additions & 0 deletions vendor/github.com/osbuild/osbuild-composer/pkg/splunk_logger/environment_hook.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ github.com/osbuild/community-gateway/oidc-authorizer/pkg/identity
# github.com/osbuild/images v0.73.0
## explicit; go 1.21
github.com/osbuild/images/pkg/crypt
# github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240311100454-57ebfb401131
## explicit; go 1.19
# github.com/osbuild/osbuild-composer/pkg/splunk_logger v0.0.0-20240814102216-0239db53236d
## explicit; go 1.21
github.com/osbuild/osbuild-composer/pkg/splunk_logger
# github.com/perimeterx/marshmallow v1.1.5
## explicit; go 1.17
Expand Down
Loading