-
Notifications
You must be signed in to change notification settings - Fork 355
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 lakefs instrumentation #6217
Changes from 5 commits
bca631f
6858929
297ae9e
d18948b
ec65f50
a54092a
99da604
258500b
0715d58
b610d8e
0d19140
534a501
4f2122d
fa0bed9
c666bf7
9eedc7e
e9b3ae9
acce1d0
25bd1c9
9c05160
0268f29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,7 +11,9 @@ import ( | |||||||||||||
"github.com/mitchellh/go-homedir" | ||||||||||||||
"github.com/spf13/cobra" | ||||||||||||||
"github.com/spf13/viper" | ||||||||||||||
"github.com/treeverse/lakefs/pkg/block" | ||||||||||||||
"github.com/treeverse/lakefs/pkg/config" | ||||||||||||||
"github.com/treeverse/lakefs/pkg/kv/local" | ||||||||||||||
"github.com/treeverse/lakefs/pkg/logging" | ||||||||||||||
"github.com/treeverse/lakefs/pkg/version" | ||||||||||||||
) | ||||||||||||||
|
@@ -41,26 +43,50 @@ func init() { | |||||||||||||
rootCmd.PersistentFlags().StringVarP(&cfgFile, "config", "c", "", "config file (default is $HOME/.lakefs.yaml)") | ||||||||||||||
rootCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle") | ||||||||||||||
rootCmd.PersistentFlags().Bool(config.UseLocalConfiguration, false, "Use lakeFS local default configuration") | ||||||||||||||
rootCmd.PersistentFlags().Bool(config.QuickStartConfiguration, false, "Use lakeFS quickstart configuration") | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
func useLocal() bool { | ||||||||||||||
res, err := rootCmd.PersistentFlags().GetBool(config.UseLocalConfiguration) | ||||||||||||||
func validateQuickstartEnv(cfg *config.Config) { | ||||||||||||||
if cfg.Database.Type != local.DriverName || cfg.Blockstore.Type != block.BlockstoreTypeLocal { | ||||||||||||||
fmt.Printf("quickstart mode can only run with local settings\n") | ||||||||||||||
os.Exit(1) | ||||||||||||||
} | ||||||||||||||
accessKey := "AKIAIOSFOLQUICKSTART" | ||||||||||||||
secretKey := "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will need to lint ignore - gosec There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
cfg.Installation.UserName = "quickstart" | ||||||||||||||
cfg.Installation.AccessKeyID = config.SecureString(accessKey) | ||||||||||||||
cfg.Installation.SecretAccessKey = config.SecureString(secretKey) | ||||||||||||||
fmt.Printf("Access Key ID : %s \n", accessKey) | ||||||||||||||
fmt.Printf("Secret Access Key: %s \n", secretKey) | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
func useConfig(cfg string) bool { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
res, err := rootCmd.PersistentFlags().GetBool(cfg) | ||||||||||||||
if err != nil { | ||||||||||||||
fmt.Printf("%s: %s\n", config.UseLocalConfiguration, err) | ||||||||||||||
fmt.Printf("%s: %s\n", cfg, err) | ||||||||||||||
os.Exit(1) | ||||||||||||||
} | ||||||||||||||
if res { | ||||||||||||||
printLocalWarning(os.Stderr, "local parameters configuration") | ||||||||||||||
printLocalWarning(os.Stderr, fmt.Sprintf("%s parameters configuration", cfg)) | ||||||||||||||
} | ||||||||||||||
return res | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
func newConfig() (*config.Config, error) { | ||||||||||||||
if useLocal() { | ||||||||||||||
return config.NewLocalConfig() | ||||||||||||||
} | ||||||||||||||
quickStart := useConfig(config.QuickStartConfiguration) | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to verify that not both are on? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure we wan't to enforce mutual exclusion - do you see an issue with it? |
||||||||||||||
|
||||||||||||||
return config.NewConfig() | ||||||||||||||
if useConfig(config.UseLocalConfiguration) || quickStart { | ||||||||||||||
cfg, err := config.NewLocalConfig() | ||||||||||||||
if err != nil { | ||||||||||||||
return nil, err | ||||||||||||||
} | ||||||||||||||
if quickStart { | ||||||||||||||
validateQuickstartEnv(cfg) | ||||||||||||||
} | ||||||||||||||
return cfg, err | ||||||||||||||
} else { | ||||||||||||||
return config.NewConfig() | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
func loadConfig() *config.Config { | ||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package auth_test | ||
|
||
import ( | ||
"context" | ||
"os" | ||
"strconv" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/require" | ||
"github.com/treeverse/lakefs/pkg/auth" | ||
"github.com/treeverse/lakefs/pkg/auth/model" | ||
"github.com/treeverse/lakefs/pkg/kv/kvtest" | ||
) | ||
|
||
func validateInstrumentation(t *testing.T, ctx context.Context, mgr *auth.KVMetadataManager, inst string, k8s, docker bool) { | ||
md, err := mgr.GetMetadata(ctx) | ||
require.NoError(t, err) | ||
require.Equal(t, inst, md["instrumentation"]) | ||
require.Equal(t, strconv.FormatBool(k8s), md["is_k8s"]) | ||
require.Equal(t, strconv.FormatBool(docker), md["is_docker"]) | ||
} | ||
|
||
func TestInstrumentation(t *testing.T) { | ||
ctx := context.Background() | ||
kvStore := kvtest.GetStore(ctx, t) | ||
|
||
_ = kvStore.Set(ctx, []byte(model.PartitionKey), []byte(model.MetadataKeyPath(auth.SetupTimestampKeyName)), []byte(time.Now().Format(time.RFC3339))) | ||
mgr := auth.NewKVMetadataManager("test", "12345", "mem", kvStore) | ||
|
||
// No env vars - defaults to run | ||
validateInstrumentation(t, ctx, mgr, auth.InstrumentationRun, false, false) | ||
|
||
// Add quickstart env var with wrong value | ||
require.NoError(t, os.Setenv("QUICKSTART", "fff")) | ||
validateInstrumentation(t, ctx, mgr, auth.InstrumentationRun, false, false) | ||
|
||
// Add docker env file | ||
auth.DockeEnvExists = "" | ||
validateInstrumentation(t, ctx, mgr, auth.InstrumentationRun, false, true) | ||
|
||
// Add sample repo env var | ||
require.NoError(t, os.Setenv("LAKEFS_ACCESS_KEY_ID", "LKFSSAMPLES")) | ||
validateInstrumentation(t, ctx, mgr, auth.InstrumentationSamplesRepo, false, true) | ||
|
||
// Add quickstart env var with right value | ||
require.NoError(t, os.Setenv("LAKEFS_ACCESS_KEY_ID", "QUICKSTART")) | ||
validateInstrumentation(t, ctx, mgr, auth.InstrumentationQuickstart, false, true) | ||
|
||
// Add K8s env var | ||
require.NoError(t, os.Setenv("KUBERNETES_SERVICE_HOST", "")) | ||
validateInstrumentation(t, ctx, mgr, auth.InstrumentationQuickstart, true, true) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name is a bit misleading as we verify the database and blockstore - but we override the configuration and not set them as default.
If we like to verify that the default as not override we should do the same as local - setup the right defaults for quick-start and verify after the configuration is loaded that the user didn't override the default for all the values we require.