purger: fix request to purger nginx path and returned purged instances

internal/pkg/rpaas/k8s.go

@@ -990,15 +990,19 @@ func (m *k8sRpaasManager) PurgeCache(ctx context.Context, instanceName string, a
 	port := util.PortByName(nginx.Spec.PodTemplate.Ports, nginxManager.PortNameManagement)
 	var purgeErrors error
 	purgeCount := 0
+	status := false
 	for _, podStatus := range podMap {
 		if !podStatus.Running {
 			continue
 		}
-		if err = m.cacheManager.PurgeCache(podStatus.Address, args.Path, port, args.PreservePath); err != nil {
+		status, err = m.cacheManager.PurgeCache(podStatus.Address, args.Path, port, args.PreservePath)
+		if err != nil {
 			purgeErrors = multierror.Append(purgeErrors, errors.Wrapf(err, "pod %s failed", podStatus.Address))
 			continue
 		}
-		purgeCount++
+		if status {
+			purgeCount++
+		}
 	}
 	return purgeCount, purgeErrors
 }

internal/pkg/rpaas/k8s_test.go

@@ -80,14 +80,14 @@ EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
 )
 
 type fakeCacheManager struct {
-	purgeCacheFunc func(host, path string, port int32, preservePath bool) error
+	purgeCacheFunc func(host, path string, port int32, preservePath bool) (bool, error)
 }
 
-func (f fakeCacheManager) PurgeCache(host, path string, port int32, preservePath bool) error {
+func (f fakeCacheManager) PurgeCache(host, path string, port int32, preservePath bool) (bool, error) {
 	if f.purgeCacheFunc != nil {
 		return f.purgeCacheFunc(host, path, port, preservePath)
 	}
-	return nil
+	return false, nil
 }
 
 func Test_k8sRpaasManager_DeleteBlock(t *testing.T) {
@@ -1765,25 +1765,25 @@ func Test_k8sRpaasManager_PurgeCache(t *testing.T) {
 			},
 		},
 		{
-			name:         "return the number of nginx instances where cache was purged",
+			name:         "return the number of unsuccessfully purged instances",
 			instance:     "my-instance",
 			args:         PurgeCacheArgs{Path: "/index.html"},
 			cacheManager: fakeCacheManager{},
 			assertion: func(t *testing.T, count int, err error) {
 				assert.NoError(t, err)
-				assert.Equal(t, 2, count)
+				assert.Equal(t, 0, count)
 			},
 		},
 		{
 			name:     "return the number of nginx instances where cache was purged and error",
 			instance: "my-instance",
 			args:     PurgeCacheArgs{Path: "/index.html"},
 			cacheManager: fakeCacheManager{
-				purgeCacheFunc: func(host, path string, port int32, preservePath bool) error {
+				purgeCacheFunc: func(host, path string, port int32, preservePath bool) (bool, error) {
 					if host == "10.0.0.9" {
-						return nginxManager.NginxError{Msg: "some nginx error"}
+						return false, nginxManager.NginxError{Msg: "some nginx error"}
 					}
-					return nil
+					return true, nil
 				},
 			},
 			assertion: func(t *testing.T, count int, err error) {

internal/pkg/rpaas/manager.go

@@ -151,7 +151,7 @@ type BindAppArgs struct {
 }
 
 type CacheManager interface {
-	PurgeCache(host, path string, port int32, preservePath bool) error
+	PurgeCache(host, path string, port int32, preservePath bool) (bool, error)
 }
 
 type PurgeCacheArgs struct {

internal/pkg/rpaas/nginx/manager.go

@@ -54,45 +54,57 @@ func vtsLocationMatch() string {
 	return defaultVTSLocationMatch
 }
 
-func (m NginxManager) PurgeCache(host, purgePath string, port int32, preservePath bool) error {
+func (m NginxManager) PurgeCache(host, purgePath string, port int32, preservePath bool) (bool, error) {
+	purged := false
 	for _, encoding := range []string{"gzip", "identity"} {
 		headers := map[string]string{"Accept-Encoding": encoding}
-
+		separator := "/"
+		if strings.HasPrefix(purgePath, "/") {
+			separator = ""
+		}
 		if preservePath {
-			separator := "/"
-			if strings.HasPrefix(purgePath, "/") {
-				separator = ""
-			}
 			path := fmt.Sprintf("%s%s%s", defaultPurgeLocation, separator, purgePath)
-			if err := m.purgeRequest(host, path, port, headers); err != nil {
-				return err
+			status, err := m.purgeRequest(host, path, port, headers)
+			if err != nil {
+				return false, err
+			}
+			if status {
+				purged = true
 			}
 		} else {
 			for _, scheme := range []string{"http", "https"} {
-				path := fmt.Sprintf("%s/%s%s", defaultPurgeLocation, scheme, purgePath)
-				if err := m.purgeRequest(host, path, port, headers); err != nil {
-					return err
+				path := fmt.Sprintf("%s/%s%s%s", defaultPurgeLocation, scheme, separator, purgePath)
+				status, err := m.purgeRequest(host, path, port, headers)
+				if err != nil {
+					return false, err
+				}
+				if status {
+					purged = true
 				}
 			}
 		}
+
 	}
-	return nil
+	return purged, nil
 }
 
-func (m NginxManager) purgeRequest(host, path string, port int32, headers map[string]string) error {
+func (m NginxManager) purgeRequest(host, path string, port int32, headers map[string]string) (bool, error) {
 	resp, err := m.requestNginx(host, path, port, headers)
 	if err != nil {
 		errorMessage := fmt.Sprintf("cannot purge nginx cache - error requesting nginx server: %v", err)
 		logrus.Error(errorMessage)
-		return NginxError{Msg: errorMessage}
+		return false, NginxError{Msg: errorMessage}
 	}
-	// StatusNotFound is a valid response when nginx does not have the path on its cache.
-	if resp.StatusCode == http.StatusOK || resp.StatusCode == http.StatusNotFound {
-		return nil
+	switch resp.StatusCode {
+	case http.StatusOK:
+		return true, nil
+	case http.StatusNotFound:
+		return false, nil
+	default:
+		errorMessage := fmt.Sprintf("cannot purge nginx cache - unexpected response from nginx server: %d", resp.StatusCode)
+		logrus.Error(errorMessage)
+		return false, NginxError{Msg: errorMessage}
 	}
-	errorMessage := fmt.Sprintf("cannot purge nginx cache - unexpected response from nginx server: %d", resp.StatusCode)
-	logrus.Error(errorMessage)
-	return NginxError{Msg: errorMessage}
 }
 
 func (m NginxManager) requestNginx(host, path string, port int32, headers map[string]string) (*http.Response, error) {

internal/pkg/rpaas/nginx/manager_test.go

@@ -11,6 +11,7 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -21,6 +22,7 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 		preservePath  bool
 		assertion     func(*testing.T, error)
 		nginxResponse http.HandlerFunc
+		status        bool
 	}{
 		{
 			description:  "returns not found error when nginx returns 404 and preservePath is false",
@@ -32,6 +34,7 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 			nginxResponse: func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusNotFound)
 			},
+			status: false,
 		},
 		{
 			description:  "returns not found error when nginx returns 404 and preservePath is true",
@@ -43,6 +46,7 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 			nginxResponse: func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusNotFound)
 			},
+			status: false,
 		},
 		{
 			description:  "returns not found error when nginx returns 500",
@@ -54,6 +58,7 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 			nginxResponse: func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusInternalServerError)
 			},
+			status: false,
 		},
 		{
 			description:  "makes a request to /purge/<purgePath> when preservePath is true",
@@ -69,6 +74,7 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 					w.WriteHeader(http.StatusNotFound)
 				}
 			},
+			status: true,
 		},
 		{
 			description:  "makes a request to /purge/<purgePath> when preservePath is true with custom cache key",
@@ -84,6 +90,7 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 					w.WriteHeader(http.StatusNotFound)
 				}
 			},
+			status: true,
 		},
 		{
 			description:  "makes a request to /purge/<protocol>/<purgePath> when preservePath is false",
@@ -99,6 +106,7 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 					w.WriteHeader(http.StatusNotFound)
 				}
 			},
+			status: true,
 		},
 		{
 			description:  "requests with gzip and identity values for Accept-Encoding header when preservePath is true",
@@ -108,12 +116,13 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 				require.NoError(t, err)
 			},
 			nginxResponse: func(w http.ResponseWriter, r *http.Request) {
-				if r.Header.Get("Accept-Encoding") == "gzip" || r.Header.Get("Accept-Encoding") == "identity" {
+				if (r.Header.Get("Accept-Encoding") == "gzip" || r.Header.Get("Accept-Encoding") == "identity") && r.RequestURI == "/purge/index.html" {
 					w.WriteHeader(http.StatusOK)
 				} else {
 					w.WriteHeader(http.StatusNotAcceptable)
 				}
 			},
+			status: true,
 		},
 		{
 			description:  "requests with gzip and identity values for Accept-Encoding header when preservePath is false",
@@ -123,12 +132,13 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 				require.NoError(t, err)
 			},
 			nginxResponse: func(w http.ResponseWriter, r *http.Request) {
-				if r.Header.Get("Accept-Encoding") == "gzip" || r.Header.Get("Accept-Encoding") == "identity" {
+				if (r.Header.Get("Accept-Encoding") == "gzip" || r.Header.Get("Accept-Encoding") == "identity") && (r.RequestURI == "/purge/http/index.html" || r.RequestURI == "/purge/https/index.html") {
 					w.WriteHeader(http.StatusOK)
 				} else {
 					w.WriteHeader(http.StatusNotAcceptable)
 				}
 			},
+			status: true,
 		},
 	}
 
@@ -143,8 +153,9 @@ func TestNginxManager_PurgeCache(t *testing.T) {
 			port, err := strconv.ParseUint(url.Port(), 10, 16)
 			require.NoError(t, err)
 
-			err = nginx.PurgeCache(url.Hostname(), tt.purgePath, int32(port), tt.preservePath)
+			purgeStatus, err := nginx.PurgeCache(url.Hostname(), tt.purgePath, int32(port), tt.preservePath)
 			tt.assertion(t, err)
+			assert.Equal(t, tt.status, purgeStatus)
 		})
 	}
 }

internal/purge/cache.go

@@ -84,15 +84,18 @@ func (p *PurgeAPI) PurgeCache(ctx context.Context, name string, args rpaas.Purge
 
 	var purgeErrors error
 	purgeCount := 0
+	status := false
 	for _, pod := range pods {
 		if !pod.Running {
 			continue
 		}
-		if err = p.cacheManager.PurgeCache(pod.Address, args.Path, port, args.PreservePath); err != nil {
+		if status, err = p.cacheManager.PurgeCache(pod.Address, args.Path, port, args.PreservePath); err != nil {
 			purgeErrors = multierror.Append(purgeErrors, errors.Wrapf(err, "pod %s:%d failed", pod.Address, port))
 			continue
 		}
-		purgeCount++
+		if status {
+			purgeCount++
+		}
 	}
 	return purgeCount, purgeErrors
 }

internal/purge/cache_test.go

@@ -24,14 +24,14 @@ func bodyContent(rsp *httptest.ResponseRecorder) string {
 }
 
 type fakeCacheManager struct {
-	purgeCacheFunc func(host, path string, port int32, preservePath bool) error
+	purgeCacheFunc func(host, path string, port int32, preservePath bool) (bool, error)
 }
 
-func (f fakeCacheManager) PurgeCache(host, path string, port int32, preservePath bool) error {
+func (f fakeCacheManager) PurgeCache(host, path string, port int32, preservePath bool) (bool, error) {
 	if f.purgeCacheFunc != nil {
 		return f.purgeCacheFunc(host, path, port, preservePath)
 	}
-	return nil
+	return false, nil
 }
 
 func TestCachePurge(t *testing.T) {
@@ -49,6 +49,18 @@ func TestCachePurge(t *testing.T) {
 			requestBody:    `{"path":"/index.html","preserve_path":true}`,
 			expectedStatus: http.StatusOK,
 			expectedBody:   `{"path":"/index.html","instances_purged":2}`,
+			cacheManager: fakeCacheManager{
+				purgeCacheFunc: func(host, path string, port int32, preservePath bool) (bool, error) {
+					return true, nil
+				},
+			},
+		},
+		{
+			name:           "no cache key found",
+			instance:       "sample-rpaasv2",
+			requestBody:    `{"path":"/index.html","preserve_path":true}`,
+			expectedStatus: http.StatusOK,
+			expectedBody:   `{"path":"/index.html"}`,
 			cacheManager:   fakeCacheManager{},
 		},
 		{
@@ -58,11 +70,11 @@ func TestCachePurge(t *testing.T) {
 			expectedStatus: http.StatusOK,
 			expectedBody:   `{"path":"/index.html","instances_purged":1,"error":"1 error occurred:\n\t* pod 172.0.2.2:8889 failed: some nginx error\n\n"}`,
 			cacheManager: fakeCacheManager{
-				purgeCacheFunc: func(host, path string, port int32, preservePath bool) error {
+				purgeCacheFunc: func(host, path string, port int32, preservePath bool) (bool, error) {
 					if host == "172.0.2.2" {
-						return nginxManager.NginxError{Msg: "some nginx error"}
+						return false, nginxManager.NginxError{Msg: "some nginx error"}
 					}
-					return nil
+					return true, nil
 				},
 			},
 		},
@@ -121,7 +133,11 @@ func TestCachePurgeBulk(t *testing.T) {
 			requestBody:    `[{"path":"/index.html","preserve_path":true}]`,
 			expectedStatus: http.StatusOK,
 			expectedBody:   `[{"path":"/index.html","instances_purged":2}]`,
-			cacheManager:   fakeCacheManager{},
+			cacheManager: fakeCacheManager{
+				purgeCacheFunc: func(host, path string, port int32, preservePath bool) (bool, error) {
+					return true, nil
+				},
+			},
 		},
 		{
 			name:           "fails on some servers",
@@ -130,11 +146,11 @@ func TestCachePurgeBulk(t *testing.T) {
 			expectedStatus: http.StatusInternalServerError,
 			expectedBody:   `[{"path":"/index.html","instances_purged":2},{"path":"/other.html","instances_purged":1,"error":"1 error occurred:\n\t* pod 172.0.2.2:8889 failed: some nginx error\n\n"}]`,
 			cacheManager: fakeCacheManager{
-				purgeCacheFunc: func(host, path string, port int32, preservePath bool) error {
+				purgeCacheFunc: func(host, path string, port int32, preservePath bool) (bool, error) {
 					if host == "172.0.2.2" && path == "/other.html" {
-						return nginxManager.NginxError{Msg: "some nginx error"}
+						return false, nginxManager.NginxError{Msg: "some nginx error"}
 					}
-					return nil
+					return true, nil
 				},
 			},
 		},