Repository: kubermatic/machine-controller
Pull request #706 (2020-03-11 08:51:58)

Enable redhat subscriptions pagination
**What this PR does / why we need it**:
Support getting red hat subscriptions paginations for rhsm. 
  
**Which issue(s) this PR fixes**
Fixes kubermatic/kubermatic#5200

```release-note
None
```


kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/706#" title="Author self-approved">moadqassem</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/master/OWNERS)~~ [moadqassem]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
xrstf: /approve
/lgtm
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: 4edf217f213a6ffa7f4fa403df703e769b1a780b</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/706#" title="Author self-approved">moadqassem</a>*, *<a href="https://github.com/kubermatic/machine-controller/pull/706#issuecomment-597562684" title="LGTM">xrstf</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/master/OWNERS)~~ [moadqassem,xrstf]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/cloudprovider/rhsm/subscription_manager.go b/pkg/cloudprovider/rhsm/subscription_manager.go
index 7e004951..02a9fcb9 100644
--- a/pkg/cloudprovider/rhsm/subscription_manager.go
+++ b/pkg/cloudprovider/rhsm/subscription_manager.go
@@ -55,11 +55,12 @@ type credentials struct {
 }
 
 type defaultRedHatSubscriptionManager struct {
-	offlineToken string
-	authURL      string
-	apiURL       string
-	client       *http.Client
-	credentials  *credentials
+	offlineToken    string
+	authURL         string
+	apiURL          string
+	client          *http.Client
+	credentials     *credentials
+	requestsLimiter int
 }
 
 var errUnauthenticatedRequest = errors.New("unauthenticated")
@@ -73,9 +74,10 @@ func NewRedHatSubscriptionManager(offlineToken string) (RedHatSubscriptionManage
 		client: &http.Client{
 			Timeout: 10 * time.Second,
 		},
-		authURL:      "https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token",
-		apiURL:       "https://api.access.redhat.com/management/v1/systems",
-		offlineToken: offlineToken,
+		authURL:         "https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token",
+		apiURL:          "https://api.access.redhat.com/management/v1/systems",
+		offlineToken:    offlineToken,
+		requestsLimiter: 100,
 	}, nil
 }
 
@@ -164,77 +166,94 @@ func (d *defaultRedHatSubscriptionManager) refreshToken() error {
 		return nil
 	}
 
-	return fmt.Errorf("error while exeucting request with status code: %v and message: %s", res.StatusCode, string(data))
+	return fmt.Errorf("error while executing request with status code: %v and message: %s", res.StatusCode, string(data))
 }
 
 func (d *defaultRedHatSubscriptionManager) findSystemsProfile(name string) (string, error) {
-	req, err := http.NewRequest("GET", d.apiURL, nil)
+	var offset int
+	for {
+		systemsInfo, err := d.executeFindSystemsRequest(offset)
+		if err != nil {
+			return "", fmt.Errorf("failed to retrieve systems: %v", err)
+		}
+
+		for _, system := range systemsInfo.Body {
+			if system.Name == name {
+				return system.UUID, nil
+			}
+		}
+
+		if len(systemsInfo.Body) < d.requestsLimiter {
+			break
+		}
+
+		offset += len(systemsInfo.Body)
+	}
+
+	klog.Infof("no machine name %s is found", name)
+	return "", nil
+}
+
+func (d *defaultRedHatSubscriptionManager) deleteSubscription(uuid string) error {
+	req, err := http.NewRequest("DELETE", fmt.Sprintf("%s/%s", d.apiURL, uuid), nil)
 	if err != nil {
-		return "", fmt.Errorf("failed to create fetch systems request: %v", err)
+		return fmt.Errorf("failed to create delete system request: %v", err)
 	}
 
 	req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", d.credentials.AccessToken))
 
 	res, err := d.client.Do(req)
 	if err != nil {
-		return "", fmt.Errorf("failed executing fetch systems request: %v", err)
+		return fmt.Errorf("faild to delete systsem profile: %v", err)
 	}
 	defer res.Body.Close()
 
 	data, err := ioutil.ReadAll(res.Body)
 	if err != nil {
-		return "", fmt.Errorf("failed while reading response: %v", err)
+		return fmt.Errorf("failed while reading response: %v", err)
 	}
 
-	if res.StatusCode != http.StatusOK {
+	if res.StatusCode != http.StatusNoContent {
 		if res.StatusCode == http.StatusUnauthorized {
-			return "", errUnauthenticatedRequest
+			return errUnauthenticatedRequest
 		}
-		return "", fmt.Errorf("error while exeucting request with status code: %v and message: %s", res.StatusCode, string(data))
-	}
-
-	var fetchedSystems = &systemsResponse{}
-	if err := json.Unmarshal(data, fetchedSystems); err != nil {
-		return "", fmt.Errorf("failed while unmarshalling data: %v", err)
-	}
 
-	// TODO(MQ): add logic to iterate over all pages.
-	for _, system := range fetchedSystems.Body {
-		if system.Name == name {
-			return system.UUID, nil
-		}
+		return fmt.Errorf("error while executing request with status code: %v and message: %s", res.StatusCode, string(data))
 	}
 
-	klog.Infof("no machine name %s is found", name)
-	return "", nil
+	return nil
 }
 
-func (d *defaultRedHatSubscriptionManager) deleteSubscription(uuid string) error {
-	req, err := http.NewRequest("DELETE", fmt.Sprintf("%s/%s", d.apiURL, uuid), nil)
+func (d *defaultRedHatSubscriptionManager) executeFindSystemsRequest(offset int) (*systemsResponse, error) {
+	req, err := http.NewRequest("GET", fmt.Sprintf(d.apiURL+"?limit=%v&offset=%v", d.requestsLimiter, offset), nil)
 	if err != nil {
-		return fmt.Errorf("failed to create delete system request: %v", err)
+		return nil, fmt.Errorf("failed to create fetch systems request: %v", err)
 	}
 
 	req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", d.credentials.AccessToken))
 
 	res, err := d.client.Do(req)
 	if err != nil {
-		return fmt.Errorf("faild to delete systsem profile: %v", err)
+		return nil, fmt.Errorf("failed executing fetch systems request: %v", err)
 	}
 	defer res.Body.Close()
 
 	data, err := ioutil.ReadAll(res.Body)
 	if err != nil {
-		return fmt.Errorf("failed while reading response: %v", err)
+		return nil, fmt.Errorf("failed while reading response: %v", err)
 	}
 
-	if res.StatusCode != http.StatusNoContent {
+	if res.StatusCode != http.StatusOK {
 		if res.StatusCode == http.StatusUnauthorized {
-			return errUnauthenticatedRequest
+			return nil, errUnauthenticatedRequest
 		}
+		return nil, fmt.Errorf("error while executing request with status code: %v and message: %s", res.StatusCode, string(data))
+	}
 
-		return fmt.Errorf("error while exeucting request with status code: %v and message: %s", res.StatusCode, string(data))
+	var fetchedSystems = &systemsResponse{}
+	if err := json.Unmarshal(data, fetchedSystems); err != nil {
+		return nil, fmt.Errorf("failed while unmarshalling data: %v", err)
 	}
 
-	return nil
+	return fetchedSystems, nil
 }


Test patch:
diff --git a/pkg/cloudprovider/rhsm/subscription_manager_test.go b/pkg/cloudprovider/rhsm/subscription_manager_test.go
index a750d935..df06a549 100644
--- a/pkg/cloudprovider/rhsm/subscription_manager_test.go
+++ b/pkg/cloudprovider/rhsm/subscription_manager_test.go
@@ -30,14 +30,25 @@ var (
 
 func TestDefaultRedHatSubscriptionManager_UnregisterInstance(t *testing.T) {
 	testCases := []struct {
-		name          string
-		offlineToken  string
-		testingServer *httptest.Server
+		name           string
+		requestLimiter int
+		offlineToken   string
+		testingServer  *httptest.Server
+		machineName    string
 	}{
 		{
-			name:          "execute redhat system unregister instance",
-			offlineToken:  "test_token",
-			testingServer: createTestingServer(),
+			name:           "execute redhat system unregister instance",
+			requestLimiter: 2,
+			offlineToken:   "test_token",
+			testingServer:  createTestingServer(false, false),
+			machineName:    "test-machine",
+		},
+		{
+			name:           "execute redhat system unregister instance for 5 systems",
+			requestLimiter: 2,
+			offlineToken:   "test_token",
+			testingServer:  createTestingServer(true, true),
+			machineName:    "test-machine-5",
 		},
 	}
 
@@ -52,24 +63,53 @@ func TestDefaultRedHatSubscriptionManager_UnregisterInstance(t *testing.T) {
 			}
 			manager.(*defaultRedHatSubscriptionManager).apiURL = tt.testingServer.URL + apiPath
 			manager.(*defaultRedHatSubscriptionManager).authURL = tt.testingServer.URL + authPath
+			manager.(*defaultRedHatSubscriptionManager).requestsLimiter = tt.requestLimiter
 
-			if err := manager.UnregisterInstance("test-machine-mqasim"); err != nil {
+			if err := manager.UnregisterInstance(tt.machineName); err != nil {
 				t.Fatalf("failed executing test: %v", err)
 			}
 		})
 	}
 }
 
-func createTestingServer() *httptest.Server {
-	machineUUID := "4a3ee8d7-337d-4cef-a20c-dda011f28f95"
+func createTestingServer(pagination, rounded bool) *httptest.Server {
+	var (
+		processedRequest = 1
+		result           string
+	)
 	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch r.URL.Path {
 		case authPath:
 			fmt.Fprintln(w, "{\"access_token\":\"test-access-token\"}")
 		case apiPath:
-			fmt.Fprintln(w, "{\"pagination\": {\"offset\": 0, \"limit\": 100,\"count\": 1}, \"body\": ["+
-				"{\"name\": \"test-machine-mqasim\", \"uuid\": \""+machineUUID+"\"}]}")
-		case apiPath + "/" + machineUUID:
+			if pagination {
+				switch processedRequest {
+				case 1:
+					result = "{\"pagination\": {\"offset\": 0, \"limit\": 2,\"count\": 5}, \"body\": [" +
+						"{\"name\": \"test-machine-1\", \"uuid\": \"4a3ee8d7-337d-4cef-a20c-dda011f28f96\"}," +
+						"{\"name\": \"test-machine-2\", \"uuid\": \"4a3ee8d7-337d-4cef-a20c-dda011f28f91\"}" +
+						"]}"
+				case 2:
+					result = "{\"pagination\": {\"offset\": 0, \"limit\": 2,\"count\": 5}, \"body\": [" +
+						"{\"name\": \"test-machine-3\", \"uuid\": \"4a3ee8d7-337d-4cef-a20c-dda011f28f98\"}," +
+						"{\"name\": \"test-machine-4\", \"uuid\": \"4a3ee8d7-337d-4cef-a20c-dda011f28f95\"}" +
+						"]}"
+				case 3:
+					result = "{\"pagination\": {\"offset\": 0, \"limit\": 2,\"count\": 5}, \"body\": [" +
+						"{\"name\": \"test-machine-5\", \"uuid\": \"4a3ee8d7-337d-4cef-a20c-dda011f28f99\"}" +
+						"]}"
+				}
+				processedRequest++
+				fmt.Fprint(w, result)
+			} else {
+				processedRequest++
+				fmt.Fprintln(w, "{\"pagination\": {\"offset\": 0, \"limit\": 100,\"count\": 1}, \"body\": ["+
+					"{\"name\": \"test-machine\", \"uuid\": \"4a3ee8d7-337d-4cef-a20c-dda011f28f95\"}]}")
+			}
+		case apiPath + "/4a3ee8d7-337d-4cef-a20c-dda011f28f95":
+			w.WriteHeader(http.StatusNoContent)
+			fmt.Fprint(w, "success")
+		case apiPath + "/4a3ee8d7-337d-4cef-a20c-dda011f28f99":
 			w.WriteHeader(http.StatusNoContent)
 			fmt.Fprint(w, "success")
 		}
diff --git a/test/e2e/provisioning/all_e2e_test.go b/test/e2e/provisioning/all_e2e_test.go
index b38db577..29b146fc 100644
--- a/test/e2e/provisioning/all_e2e_test.go
+++ b/test/e2e/provisioning/all_e2e_test.go
@@ -151,7 +151,7 @@ func TestAWSProvisioningE2E(t *testing.T) {
 	if len(awsKeyID) == 0 || len(awsSecret) == 0 {
 		t.Fatal("unable to run the test suite, AWS_E2E_TESTS_KEY_ID or AWS_E2E_TESTS_SECRET environment variables cannot be empty")
 	}
-	excludeSelector := &scenarioSelector{osName: []string{"sles"}}
+	excludeSelector := &scenarioSelector{osName: []string{"sles", "rhel"}}
 	// act
 	params := []string{fmt.Sprintf("<< AWS_ACCESS_KEY_ID >>=%s", awsKeyID),
 		fmt.Sprintf("<< AWS_SECRET_ACCESS_KEY >>=%s", awsSecret),
@@ -246,7 +246,7 @@ func TestGCEProvisioningE2E(t *testing.T) {
 	}
 
 	// Act. GCE does not support CentOS.
-	excludeSelector := &scenarioSelector{osName: []string{"centos", "sles"}}
+	excludeSelector := &scenarioSelector{osName: []string{"centos", "sles", "rhel"}}
 	params := []string{
 		fmt.Sprintf("<< GOOGLE_SERVICE_ACCOUNT >>=%s", googleServiceAccount),
 	}