adding first part of resolving final comments

Author: miledxz

internal/mode/static/manager.go

@@ -144,7 +144,7 @@ func StartManager(cfg config.Config) error {
 		return fmt.Errorf("cannot clear NGINX configuration folders: %w", err)
 	}
 
-	var processHandler = &ngxruntime.ProcessHandlerImpl{}
+	processHandler := &ngxruntime.NewProcessHandlerImpl{}
 
 	// Ensure NGINX is running before registering metrics & starting the manager.
 	if err := processHandler.EnsureNginxRunning(ctx); err != nil {

internal/mode/static/nginx/runtime/manager.go

@@ -34,7 +34,7 @@ type (
 
 var childProcPathFmt = "/proc/%[1]v/task/%[1]v/children"
 
-//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . nginxPlusClient
+//counterfeiter:generate . nginxPlusClient
 
 type nginxPlusClient interface {
 	UpdateHTTPServers(
@@ -49,21 +49,19 @@ type nginxPlusClient interface {
 	GetUpstreams() (*ngxclient.Upstreams, error)
 }
 
-//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ProcessHandler
+//counterfeiter:generate . ProcessHandler
 
 type ProcessHandler interface {
 	FindMainProcess(
 		ctx context.Context,
-		checkFile CheckFileFunc,
-		readFile ReadFileFunc,
 		timeout time.Duration,
 	) (int, error)
-	ReadFile(file string) ([]byte, error)
+	readFile(file string) ([]byte, error)
 	Kill(pid int) error
 	EnsureNginxRunning(ctx context.Context) error
 }
 
-//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Manager
+//counterfeiter:generate . Manager
 
 // Manager manages the runtime of NGINX.
 type Manager interface {
@@ -79,9 +77,9 @@ type Manager interface {
 	GetUpstreams() (ngxclient.Upstreams, error)
 }
 
-//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . MetricsCollector
-
 // MetricsCollector is an interface for the metrics of the NGINX runtime manager.
+//
+//counterfeiter:generate . MetricsCollector
 type MetricsCollector interface {
 	IncReloadCount()
 	IncReloadErrors()
@@ -122,13 +120,13 @@ func (m *ManagerImpl) IsPlus() bool {
 func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error {
 	start := time.Now()
 	// We find the main NGINX PID on every reload because it will change if the NGINX container is restarted.
-	pid, err := m.processHandler.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout)
+	pid, err := m.processHandler.FindMainProcess(ctx, pidFileTimeout)
 	if err != nil {
 		return fmt.Errorf("failed to find NGINX main process: %w", err)
 	}
 
 	childProcFile := fmt.Sprintf(childProcPathFmt, pid)
-	previousChildProcesses, err := m.processHandler.ReadFile(childProcFile)
+	previousChildProcesses, err := m.processHandler.readFile(childProcFile)
 	if err != nil {
 		return err
 	}
@@ -191,20 +189,18 @@ func (m *ManagerImpl) GetUpstreams() (ngxclient.Upstreams, error) {
 	return *upstreams, nil
 }
 
-type ProcessHandlerImpl struct{}
+type NewProcessHandlerImpl struct{}
 
 // EnsureNginxRunning ensures NGINX is running by locating the main process.
-func (p *ProcessHandlerImpl) EnsureNginxRunning(ctx context.Context) error {
-	if _, err := p.FindMainProcess(ctx, os.Stat, os.ReadFile, pidFileTimeout); err != nil {
+func (p *NewProcessHandlerImpl) EnsureNginxRunning(ctx context.Context) error {
+	if _, err := p.FindMainProcess(ctx, pidFileTimeout); err != nil {
 		return fmt.Errorf("failed to find NGINX main process: %w", err)
 	}
 	return nil
 }
 
-func (p *ProcessHandlerImpl) FindMainProcess(
+func (p *NewProcessHandlerImpl) FindMainProcess(
 	ctx context.Context,
-	checkFile CheckFileFunc,
-	readFile ReadFileFunc,
 	timeout time.Duration,
 ) (int, error) {
 	ctx, cancel := context.WithTimeout(ctx, timeout)
@@ -215,7 +211,7 @@ func (p *ProcessHandlerImpl) FindMainProcess(
 		500*time.Millisecond,
 		true, /* poll immediately */
 		func(_ context.Context) (bool, error) {
-			_, err := checkFile(PidFile)
+			_, err := p.checkFile(PidFile)
 			if err == nil {
 				return true, nil
 			}
@@ -228,7 +224,7 @@ func (p *ProcessHandlerImpl) FindMainProcess(
 		return 0, err
 	}
 
-	content, err := readFile(PidFile)
+	content, err := p.readFile(PidFile)
 	if err != nil {
 		return 0, err
 	}
@@ -241,10 +237,10 @@ func (p *ProcessHandlerImpl) FindMainProcess(
 	return pid, nil
 }
 
-func (p *ProcessHandlerImpl) ReadFile(file string) ([]byte, error) {
-	return os.ReadFile(file)
+func (p *NewProcessHandlerImpl) readFile(file string) ([]byte, error) {
+	return p.readFile(file)
 }
 
-func (p *ProcessHandlerImpl) Kill(pid int) error {
+func (p *NewProcessHandlerImpl) Kill(pid int) error {
 	return syscall.Kill(pid, syscall.SIGHUP)
 }

internal/mode/static/nginx/runtime/manager_test.go

@@ -239,8 +239,8 @@ func TestFindMainProcess(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			g := NewWithT(t)
-			p := runtime.ProcessHandlerImpl{}
-			result, err := p.FindMainProcess(test.ctx, test.checkFile, test.readFile, 2*time.Millisecond)
+			p := runtime.NewProcessHandlerImpl{}
+			result, err := p.FindMainProcess(test.ctx, 2*time.Millisecond)
 
 			if test.expectError {
 				g.Expect(err).To(HaveOccurred())

internal/mode/static/nginx/runtime/runtimefakes/fake_process_handler.go

@@ -33,15 +33,15 @@ type nginxConfigVerifier interface {
 	EnsureConfigVersion(ctx context.Context, expectedVersion int) error
 }
 
-// VerifyClientImpl is a client for verifying the config version.
-type VerifyClientImpl struct {
+// VerifyClient is a client for verifying the config version.
+type VerifyClient struct {
 	client  *http.Client
 	timeout time.Duration
 }
 
 // NewVerifyClient returns a new client pointed at the config version socket.
-func NewVerifyClient(timeout time.Duration) *VerifyClientImpl {
-	return &VerifyClientImpl{
+func NewVerifyClient(timeout time.Duration) *VerifyClient {
+	return &VerifyClient{
 		client: &http.Client{
 			Transport: &http.Transport{
 				DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
@@ -55,7 +55,7 @@ func NewVerifyClient(timeout time.Duration) *VerifyClientImpl {
 
 // GetConfigVersion gets the version number that we put in the nginx config to verify that we're using
 // the correct config.
-func (c *VerifyClientImpl) GetConfigVersion() (int, error) {
+func (c *VerifyClient) GetConfigVersion() (int, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), c.timeout)
 	defer cancel()
 
@@ -88,7 +88,7 @@ func (c *VerifyClientImpl) GetConfigVersion() (int, error) {
 // WaitForCorrectVersion first ensures any new worker processes have been started, and then calls the config version
 // endpoint until it gets the expectedVersion, which ensures that a new worker process has been started for that config
 // version.
-func (c *VerifyClientImpl) WaitForCorrectVersion(
+func (c *VerifyClient) WaitForCorrectVersion(
 	ctx context.Context,
 	expectedVersion int,
 	childProcFile string,
@@ -119,7 +119,7 @@ func (c *VerifyClientImpl) WaitForCorrectVersion(
 	return nil
 }
 
-func (c *VerifyClientImpl) EnsureConfigVersion(ctx context.Context, expectedVersion int) error {
+func (c *VerifyClient) EnsureConfigVersion(ctx context.Context, expectedVersion int) error {
 	return wait.PollUntilContextCancel(
 		ctx,
 		25*time.Millisecond,