Skip to content

Commit

Permalink
More fixes; add some unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
sjberman committed Jan 21, 2025
1 parent da36f73 commit bbcf56f
Show file tree
Hide file tree
Showing 38 changed files with 2,137 additions and 234 deletions.
18 changes: 18 additions & 0 deletions charts/nginx-gateway-fabric/templates/tmp-nginx-agent-conf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,21 @@ data:
{{- end }}
log:
level: debug
collector:
receivers:
host_metrics:
collection_interval: 1m0s
initial_delay: 1s
scrapers:
cpu: {}
memory: {}
disk: {}
network: {}
filesystem: {}
processors:
batch: {}
exporters:
prometheus_exporter:
server:
host: "0.0.0.0"
port: 9113
35 changes: 27 additions & 8 deletions charts/nginx-gateway-fabric/templates/tmp-nginx-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,37 @@ spec:
labels:
app.kubernetes.io/name: tmp-nginx-deployment
app.kubernetes.io/instance: {{ .Release.Name }}
annotations:
{{- if .Values.metrics.enable }}
prometheus.io/scrape: "true"
prometheus.io/port: "{{ .Values.metrics.port }}"
{{- if .Values.metrics.secure }}
prometheus.io/scheme: "https"
{{- end }}
{{- end }}
spec:
initContainers:
- name: sleep # wait for a bit for control plane to be ready
image: {{ .Values.nginxGateway.image.repository }}:{{ default .Chart.AppVersion .Values.nginxGateway.image.tag }}
imagePullPolicy: {{ .Values.nginxGateway.image.pullPolicy }}
command:
- /usr/bin/gateway
- sleep
- --duration=5s
- name: init
image: {{ .Values.nginxGateway.image.repository }}:{{ default .Chart.AppVersion .Values.nginxGateway.image.tag }}
imagePullPolicy: {{ .Values.nginxGateway.image.pullPolicy }}
command:
- /usr/bin/gateway
- initialize
- --source
- /agent/nginx-agent.conf
- --destination
- /etc/nginx-agent
- --source
- /includes/main.conf
- --destination
- /etc/nginx/main-includes
{{- if .Values.nginx.plus }}
- --source
- /includes/mgmt.conf
- --nginx-plus
{{- end }}
- --destination
- /etc/nginx/main-includes
{{- end }}
env:
- name: POD_UID
valueFrom:
Expand All @@ -52,6 +59,10 @@ spec:
runAsUser: 101
runAsGroup: 1001
volumeMounts:
- name: nginx-agent-config
mountPath: /agent
- name: nginx-agent
mountPath: /etc/nginx-agent
- name: nginx-includes-bootstrap
mountPath: /includes
- name: nginx-main-includes
Expand All @@ -69,6 +80,8 @@ spec:
name: http
- containerPort: 443
name: https
- name: metrics
containerPort: {{ .Values.metrics.port }}
securityContext:
seccompProfile:
type: RuntimeDefault
Expand All @@ -83,6 +96,8 @@ spec:
volumeMounts:
- name: nginx-agent
mountPath: /etc/nginx-agent
- name: nginx-agent-log
mountPath: /var/log/nginx-agent
- name: nginx-conf
mountPath: /etc/nginx/conf.d
- name: nginx-stream-conf
Expand Down Expand Up @@ -139,8 +154,12 @@ spec:
{{- end }}
volumes:
- name: nginx-agent
emptyDir: {}
- name: nginx-agent-config
configMap:
name: nginx-agent-config
- name: nginx-agent-log
emptyDir: {}
- name: nginx-conf
emptyDir: {}
- name: nginx-stream-conf
Expand Down
27 changes: 16 additions & 11 deletions cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,14 +519,14 @@ func createInitializeCommand() *cobra.Command {

// flag values
var srcFiles []string
var dest string
var destDirs []string
var plus bool

cmd := &cobra.Command{
Use: "initialize",
Short: "Write initial configuration files",
RunE: func(_ *cobra.Command, _ []string) error {
if err := validateCopyArgs(srcFiles, dest); err != nil {
if err := validateCopyArgs(srcFiles, destDirs); err != nil {
return err
}

Expand All @@ -546,7 +546,7 @@ func createInitializeCommand() *cobra.Command {
logger.Info(
"Starting init container",
"source filenames to copy", srcFiles,
"destination directory", dest,
"destination directories", destDirs,
"nginx-plus",
plus,
)
Expand All @@ -558,16 +558,21 @@ func createInitializeCommand() *cobra.Command {
Logger: logger.WithName("deployCtxCollector"),
})

files := make([]fileToCopy, 0, len(srcFiles))
for i, src := range srcFiles {
files = append(files, fileToCopy{
destDirName: destDirs[i],
srcFileName: src,
})
}

return initialize(initializeConfig{
fileManager: file.NewStdLibOSFileManager(),
fileGenerator: ngxConfig.NewGeneratorImpl(plus, nil, logger.WithName("generator")),
logger: logger,
plus: plus,
collector: dcc,
copy: copyFiles{
srcFileNames: srcFiles,
destDirName: dest,
},
copy: files,
})
},
}
Expand All @@ -579,11 +584,11 @@ func createInitializeCommand() *cobra.Command {
"The source files to be copied",
)

cmd.Flags().StringVar(
&dest,
cmd.Flags().StringSliceVar(
&destDirs,
destFlag,
"",
"The destination directory for the source files to be copied to",
[]string{},
"The destination directories for the source files at the same array index to be copied to",
)

cmd.Flags().BoolVar(
Expand Down
12 changes: 6 additions & 6 deletions cmd/gateway/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,23 @@ const (
collectDeployCtxTimeout = 10 * time.Second
)

type copyFiles struct {
destDirName string
srcFileNames []string
type fileToCopy struct {
destDirName string
srcFileName string
}

type initializeConfig struct {
collector licensing.Collector
fileManager file.OSFileManager
fileGenerator config.Generator
logger logr.Logger
copy copyFiles
copy []fileToCopy
plus bool
}

func initialize(cfg initializeConfig) error {
for _, src := range cfg.copy.srcFileNames {
if err := copyFile(cfg.fileManager, src, cfg.copy.destDirName); err != nil {
for _, f := range cfg.copy {
if err := copyFile(cfg.fileManager, f.srcFileName, f.destDirName); err != nil {
return err
}
}
Expand Down
36 changes: 27 additions & 9 deletions cmd/gateway/initialize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,15 @@ func TestInitialize_OSS(t *testing.T) {
ic := initializeConfig{
fileManager: fakeFileMgr,
logger: zap.New(),
copy: copyFiles{
destDirName: "destDir",
srcFileNames: []string{"src1", "src2"},
copy: []fileToCopy{
{
destDirName: "destDir",
srcFileName: "src1",
},
{
destDirName: "destDir2",
srcFileName: "src2",
},
},
plus: false,
}
Expand All @@ -56,9 +62,15 @@ func TestInitialize_OSS_Error(t *testing.T) {
ic := initializeConfig{
fileManager: fakeFileMgr,
logger: zap.New(),
copy: copyFiles{
destDirName: "destDir",
srcFileNames: []string{"src1", "src2"},
copy: []fileToCopy{
{
destDirName: "destDir",
srcFileName: "src1",
},
{
destDirName: "destDir2",
srcFileName: "src2",
},
},
plus: false,
}
Expand Down Expand Up @@ -114,9 +126,15 @@ func TestInitialize_Plus(t *testing.T) {
logger: zap.New(),
collector: fakeCollector,
fileGenerator: fakeGenerator,
copy: copyFiles{
destDirName: "destDir",
srcFileNames: []string{"src1", "src2"},
copy: []fileToCopy{
{
destDirName: "destDir",
srcFileName: "src1",
},
{
destDirName: "destDir2",
srcFileName: "src2",
},
},
plus: true,
}
Expand Down
9 changes: 6 additions & 3 deletions cmd/gateway/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,15 @@ func ensureNoPortCollisions(ports ...int) error {
return nil
}

// validateCopyArgs ensures that arguments to the sleep command are set.
func validateCopyArgs(srcFiles []string, dest string) error {
// validateCopyArgs ensures that arguments to the initialize command are set.
func validateCopyArgs(srcFiles []string, destDirs []string) error {
if len(srcFiles) != len(destDirs) {
return errors.New("source and destination must have the same number of elements")
}
if len(srcFiles) == 0 {
return errors.New("source must not be empty")
}
if len(dest) == 0 {
if len(destDirs) == 0 {
return errors.New("destination must not be empty")
}

Expand Down
18 changes: 12 additions & 6 deletions cmd/gateway/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,41 +554,47 @@ func TestEnsureNoPortCollisions(t *testing.T) {
g.Expect(ensureNoPortCollisions(9113, 9113)).ToNot(Succeed())
}

func TestValidateSleepArgs(t *testing.T) {
func TestValidateInitializeArgs(t *testing.T) {
t.Parallel()

tests := []struct {
name string
dest string
destDirs []string
srcFiles []string
expErr bool
}{
{
name: "valid values",
dest: "/dest/file",
destDirs: []string{"/dest/"},
srcFiles: []string{"/src/file"},
expErr: false,
},
{
name: "invalid dest",
dest: "",
destDirs: []string{},
srcFiles: []string{"/src/file"},
expErr: true,
},
{
name: "invalid src",
dest: "/dest/file",
destDirs: []string{"/dest/"},
srcFiles: []string{},
expErr: true,
},
{
name: "different lengths",
destDirs: []string{"/dest/"},
srcFiles: []string{"src1", "src2"},
expErr: true,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
g := NewWithT(t)

err := validateCopyArgs(tc.srcFiles, tc.dest)
err := validateCopyArgs(tc.srcFiles, tc.destDirs)
if !tc.expErr {
g.Expect(err).ToNot(HaveOccurred())
} else {
Expand Down
Loading

0 comments on commit bbcf56f

Please sign in to comment.