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

cloudapi: git sha version api and journal for workers #4484

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
exclude-dirs:
- vendor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For full transparency, I sneaked this hunk in with the last amend, I realized that go linter lints also vendor directories and that slows it significantly. Can do a separate PR if requested.

linters-settings:
govet:
disable:
Expand Down
2 changes: 1 addition & 1 deletion cmd/osbuild-composer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func main() {
logLevel, err := logrus.ParseLevel(config.LogLevel)

logrus.SetReportCaller(true)
// Add context hook to log operation_id and external_id
logrus.AddHook(&common.BuildHook{})
logrus.AddHook(&common.ContextHook{})

if err == nil {
Expand Down
11 changes: 11 additions & 0 deletions cmd/osbuild-worker/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ type composerConfig struct {
Proxy string `toml:"proxy"`
}

type loggingConfig struct {
Format string `toml:"format"` // journal, text or json (defaults to journal or text depending on the OS)
Level string `toml:"level"`
NoDiscard bool `toml:"no_discard"` // do not discard stdout/stderr logs (useful for debugging)
}

type kerberosConfig struct {
Principal string `toml:"principal"`
KeyTab string `toml:"keytab"`
Expand Down Expand Up @@ -88,6 +94,7 @@ type repositoryMTLSConfig struct {

type workerConfig struct {
Composer *composerConfig `toml:"composer"`
Logging *loggingConfig `toml:"logging"`
Koji map[string]kojiServerConfig `toml:"koji"`
GCP *gcpConfig `toml:"gcp"`
Azure *azureConfig `toml:"azure"`
Expand Down Expand Up @@ -115,6 +122,10 @@ func parseConfig(file string) (*workerConfig, error) {
Type: "host",
},
DeploymentChannel: "local",
Logging: &loggingConfig{
Format: "journal",
Level: "info",
},
}

_, err := toml.DecodeFile(file, &config)
Expand Down
16 changes: 16 additions & 0 deletions cmd/osbuild-worker/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ type = "aws.ec2"
iam_profile = "osbuild-worker"
key_name = "osbuild-worker"
cloudwatch_group = "osbuild-worker"
[logging]
level = "debug"
format = "text"
`,
want: &workerConfig{
BasePath: "/api/image-builder-worker/v1",
Expand Down Expand Up @@ -137,6 +141,10 @@ cloudwatch_group = "osbuild-worker"
ServerURL: "https://example.com/pulp",
},
DeploymentChannel: "local",
Logging: &loggingConfig{
Level: "debug",
Format: "text",
},
},
},
{
Expand All @@ -148,6 +156,10 @@ cloudwatch_group = "osbuild-worker"
Type: "host",
},
DeploymentChannel: "local",
Logging: &loggingConfig{
Format: "journal",
Level: "info",
},
},
},
{
Expand All @@ -159,6 +171,10 @@ cloudwatch_group = "osbuild-worker"
Type: "host",
},
DeploymentChannel: "staging",
Logging: &loggingConfig{
Format: "journal",
Level: "info",
},
},
},
}
Expand Down
42 changes: 42 additions & 0 deletions cmd/osbuild-worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"errors"
"flag"
"fmt"
"io"
"log"
"net/url"
"os"
"path"
Expand All @@ -17,11 +19,13 @@ import (
slogger "github.com/osbuild/osbuild-composer/pkg/splunk_logger"

"github.com/BurntSushi/toml"
"github.com/coreos/go-systemd/journal"
"github.com/sirupsen/logrus"

"github.com/osbuild/images/pkg/arch"
"github.com/osbuild/images/pkg/dnfjson"
"github.com/osbuild/osbuild-composer/internal/cloud/awscloud"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/upload/azure"
"github.com/osbuild/osbuild-composer/internal/upload/koji"
"github.com/osbuild/osbuild-composer/internal/upload/oci"
Expand Down Expand Up @@ -168,6 +172,11 @@ func RequestAndRunJob(client *worker.Client, acceptedJobTypes []string, jobImpls
}

var run = func() {
// Redirect Go standard logger into logrus before it's used by other packages
log.SetOutput(common.Logger())
// Ensure the Go standard logger does not have any prefix or timestamp
log.SetFlags(0)

var unix bool
flag.BoolVar(&unix, "unix", false, "Interpret 'address' as a path to a unix domain socket instead of a network address")

Expand All @@ -189,6 +198,39 @@ var run = func() {
logrus.Fatalf("Could not load config file '%s': %v", configFile, err)
}

logrus.SetReportCaller(true)
logrus.AddHook(&common.BuildHook{})
logrus.SetOutput(os.Stdout)
logLevel, err := logrus.ParseLevel(config.Logging.Level)
if err != nil {
logrus.Info("Failed to load loglevel from config:", err)
} else {
logrus.SetLevel(logLevel)
}

// logger configuration
switch config.Logging.Format {
case "journal", "":
// If we are running under systemd, use the journal. Otherwise,
// fallback to text formatter.
if journal.Enabled() {
logrus.Info("Switching to journal logging mode, use logging.no_discard to keep writing to standard output/error")
logrus.SetFormatter(&logrus.JSONFormatter{})
logrus.AddHook(&common.JournalHook{})
if !config.Logging.NoDiscard {
logrus.SetOutput(io.Discard)
}
} else {
logrus.SetFormatter(&logrus.TextFormatter{})
}
case "text":
logrus.SetFormatter(&logrus.TextFormatter{})
case "json":
logrus.SetFormatter(&logrus.JSONFormatter{})
default:
logrus.Infof("Failed to set logging format from config, '%s' is not a valid option", config.Logging.Format)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI this was copy-pasted from composer/main.go we already use it there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to have some common logging setup function to actually benefit from having both composer and worker in the same repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would say no, these are completely different binaries, separated by design. Little bit of copying is sometimes better than a dependency.

logrus.Info("Composer configuration:")
encoder := toml.NewEncoder(logrus.StandardLogger().WriterLevel(logrus.InfoLevel))
err = encoder.Encode(&config)
Expand Down
13 changes: 13 additions & 0 deletions internal/cloudapi/v2/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ func (b binder) Bind(i interface{}, ctx echo.Context) error {
return nil
}

func (h *apiHandlers) GetVersion(ctx echo.Context) error {
spec, err := GetSwagger()
if err != nil {
return HTTPError(ErrorFailedToLoadOpenAPISpec)
}
version := Version{
Version: spec.Info.Version,
BuildCommit: common.ToPtr(common.BuildCommit),
BuildTime: common.ToPtr(common.BuildTime),
}
return ctx.JSON(http.StatusOK, version)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have the very same code in image-builder too, this is a copy-paste literally.

func (h *apiHandlers) GetOpenapi(ctx echo.Context) error {
spec, err := GetSwagger()
if err != nil {
Expand Down
Loading
Loading