Skip to content

Commit 0bb2f6b

Browse files
tremestremes
authored
NO-JIRA: minor update of cluster transfer code & increase the code coverage (openshift#982)
1 parent 72de1cd commit 0bb2f6b

File tree

7 files changed

+189
-54
lines changed

7 files changed

+189
-54
lines changed

‎pkg/controller/operator.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,9 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller
272272
statusReporter.AddSources(scaController)
273273
go scaController.Run(ctx)
274274

275-
clusterTransferController := clustertransfer.New(ctx, kubeClient.CoreV1(), configAggregator, insightsClient)
275+
clusterTransferController := clustertransfer.New(kubeClient.CoreV1(), configAggregator, insightsClient)
276276
statusReporter.AddSources(clusterTransferController)
277-
go clusterTransferController.Run()
277+
go clusterTransferController.Run(ctx)
278278

279279
promRulesController := insights.NewPrometheusRulesController(configAggregator, controller.KubeConfig)
280280
go promRulesController.Start(ctx)

‎pkg/insights/insightsclient/insightsclient.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type Client struct {
4141
maxBytes int64
4242
metricsName string
4343
authorizer Authorizer
44-
configClient *configv1client.Clientset
44+
configClient configv1client.Interface
4545
}
4646

4747
type Authorizer interface {
@@ -89,7 +89,7 @@ func IsHttpError(err error) bool {
8989
var ErrWaitingForVersion = fmt.Errorf("waiting for the cluster version to be loaded")
9090

9191
// New creates a Client
92-
func New(client *http.Client, maxBytes int64, metricsName string, authorizer Authorizer, configClient *configv1client.Clientset) *Client {
92+
func New(client *http.Client, maxBytes int64, metricsName string, authorizer Authorizer, configClient configv1client.Interface) *Client {
9393
if client == nil {
9494
client = &http.Client{}
9595
}

‎pkg/ocm/clustertransfer/cluster_transfer.go

+30-25
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,17 @@ const (
2525
AvailableReason = "PullSecretUpdated"
2626
)
2727

28+
var (
29+
disconnectedReason = "Disconnected"
30+
noClusterTransfer = "NoClusterTransfer"
31+
moreAcceptedClusterTransfers = "MoreAcceptedClusterTransfers"
32+
dataCorrupted = "DataCorrupted"
33+
unexpectedData = "UnexpectedData"
34+
)
35+
2836
type Controller struct {
2937
controllerstatus.StatusController
3038
coreClient corev1client.CoreV1Interface
31-
ctx context.Context
3239
configurator configobserver.Interface
3340
client clusterTransferClient
3441
pullSecret *v1.Secret
@@ -39,31 +46,29 @@ type clusterTransferClient interface {
3946
}
4047

4148
// New creates new instance of the cluster transfer controller
42-
func New(ctx context.Context,
43-
coreClient corev1client.CoreV1Interface,
49+
func New(coreClient corev1client.CoreV1Interface,
4450
configurator configobserver.Interface,
4551
insightsClient clusterTransferClient) *Controller {
4652
return &Controller{
4753
StatusController: controllerstatus.New(ControllerName),
4854
coreClient: coreClient,
49-
ctx: ctx,
5055
configurator: configurator,
5156
client: insightsClient,
5257
}
5358
}
5459

5560
// Run periodically queries the OCM API and update pull-secret accordingly
56-
func (c *Controller) Run() {
61+
func (c *Controller) Run(ctx context.Context) {
5762
cfg := c.configurator.Config()
5863
endpoint := cfg.ClusterTransfer.Endpoint
5964
interval := cfg.ClusterTransfer.Interval
6065
configCh, cancel := c.configurator.ConfigChanged()
6166
defer cancel()
62-
c.requestDataAndUpdateSecret(endpoint)
67+
c.requestDataAndUpdateSecret(ctx, endpoint)
6368
for {
6469
select {
6570
case <-time.After(interval):
66-
c.requestDataAndUpdateSecret(endpoint)
71+
c.requestDataAndUpdateSecret(ctx, endpoint)
6772
case <-configCh:
6873
cfg := c.configurator.Config()
6974
interval = cfg.ClusterTransfer.Interval
@@ -74,7 +79,7 @@ func (c *Controller) Run() {
7479

7580
// requestDataAndUpdateSecret queries the provided endpoint. If there is any data
7681
// in the response then check if a secret update is required, and if so, perform the update.
77-
func (c *Controller) requestDataAndUpdateSecret(endpoint string) {
82+
func (c *Controller) requestDataAndUpdateSecret(ctx context.Context, endpoint string) {
7883
klog.Infof("checking the availability of cluster transfer. Next check is in %s", c.configurator.Config().ClusterTransfer.Interval)
7984
data, err := c.requestClusterTransferWithExponentialBackoff(endpoint)
8085
if err != nil {
@@ -88,60 +93,60 @@ func (c *Controller) requestDataAndUpdateSecret(endpoint string) {
8893
}
8994
// we are probably in disconnected environment
9095
klog.Errorf(msg)
91-
c.updateStatus(true, msg, "Disconnected", nil)
96+
c.updateStatus(true, msg, disconnectedReason, nil)
9297
return
9398
}
9499

95100
// there's no cluster transfer for the cluster - HTTP 204
96101
if len(data) == 0 {
97102
klog.Info("no available accepted cluster transfer")
98-
c.updateStatus(true, "no available cluster transfer", "NoClusterTransfer", nil)
103+
c.updateStatus(true, "no available cluster transfer", noClusterTransfer, nil)
99104
return
100105
}
101106
// deserialize the data from the OCM API
102107
ctList, err := unmarhallResponseData(data)
103108
if err != nil {
104109
msg := fmt.Sprintf("unable to deserialize the cluster transfer API response: %v", err)
105110
klog.Error(msg)
106-
c.updateStatus(false, msg, "UnexpectedData", nil)
111+
c.updateStatus(false, msg, unexpectedData, nil)
107112
return
108113
}
109-
c.checkCTListAndOptionallyUpdatePS(ctList)
114+
c.checkCTListAndOptionallyUpdatePS(ctx, ctList)
110115
}
111116

112117
// checkCTListAndOptionallyUpdatePS checks the provided cluster transfer list length,
113118
// If there is more than 1 accepted cluster transfer then log the message, update the controller status
114119
// and do nothing. If there is only one accepted cluster transfer then
115120
// check if the `pull-secret` needs to be updated. If the `pull-secret` needs to be updated then
116121
// update it and update the controller status, otherwise just update the controller status.
117-
func (c *Controller) checkCTListAndOptionallyUpdatePS(ctList *clusterTransferList) {
122+
func (c *Controller) checkCTListAndOptionallyUpdatePS(ctx context.Context, ctList *clusterTransferList) {
118123
if ctList.Total > 1 {
119124
msg := "there are more accepted cluster transfers. The pull-secret will not be updated!"
120125
klog.Infof(msg)
121-
c.updateStatus(true, msg, "MoreAcceptedClusterTransfers", nil)
126+
c.updateStatus(true, msg, moreAcceptedClusterTransfers, nil)
122127
return
123128
}
124129
// this should not happen. This is just safe check
125130
if len(ctList.Transfers) != 1 {
126131
msg := "unexpected number of cluster transfers received from the API"
127132
klog.Infof(msg)
128-
c.updateStatus(true, msg, "UnexpectedData", nil)
133+
c.updateStatus(true, msg, unexpectedData, nil)
129134
return
130135
}
131136

132137
newPullSecret := []byte(ctList.Transfers[0].Secret)
133138
var statusMsg, reason string
134139
// check if the pull-secret needs to be updated
135-
updating, err := c.isUpdateRequired(newPullSecret)
140+
updating, err := c.isUpdateRequired(ctx, newPullSecret)
136141
if err != nil {
137142
statusMsg = fmt.Sprintf("new pull-secret check failed: %v", err)
138143
klog.Errorf(statusMsg)
139-
c.updateStatus(false, statusMsg, "DataCorrupted", nil)
144+
c.updateStatus(false, statusMsg, dataCorrupted, nil)
140145
return
141146
}
142147
if updating {
143148
klog.Info("updating the pull-secret content")
144-
err = c.updatePullSecret(newPullSecret)
149+
err = c.updatePullSecret(ctx, newPullSecret)
145150
if err != nil {
146151
statusMsg = fmt.Sprintf("failed to update pull-secret: %v", err)
147152
klog.Errorf(statusMsg)
@@ -160,8 +165,8 @@ func (c *Controller) checkCTListAndOptionallyUpdatePS(ctList *clusterTransferLis
160165
}
161166

162167
// isUpdateRequired checks if an update of the pull-secret is required or not.
163-
func (c *Controller) isUpdateRequired(newData []byte) (bool, error) {
164-
pullSecret, err := c.getPullSecret()
168+
func (c *Controller) isUpdateRequired(ctx context.Context, newData []byte) (bool, error) {
169+
pullSecret, err := c.getPullSecret(ctx)
165170
if err != nil {
166171
return false, err
167172
}
@@ -180,9 +185,9 @@ func (c *Controller) isUpdateRequired(newData []byte) (bool, error) {
180185

181186
// updatePullSecret creates a JSON merge patch of existing and new pull-secret data.
182187
// The result of the patch is used for pull-secret data update.
183-
func (c *Controller) updatePullSecret(newData []byte) error {
188+
func (c *Controller) updatePullSecret(ctx context.Context, newData []byte) error {
184189
if c.pullSecret == nil {
185-
ps, err := c.getPullSecret()
190+
ps, err := c.getPullSecret(ctx)
186191
if err != nil {
187192
return err
188193
}
@@ -196,7 +201,7 @@ func (c *Controller) updatePullSecret(newData []byte) error {
196201
}
197202

198203
c.pullSecret.Data[v1.DockerConfigJsonKey] = updatedData
199-
_, err = c.coreClient.Secrets("openshift-config").Update(c.ctx, c.pullSecret, metav1.UpdateOptions{})
204+
_, err = c.coreClient.Secrets("openshift-config").Update(ctx, c.pullSecret, metav1.UpdateOptions{})
200205
if err != nil {
201206
return err
202207
}
@@ -260,8 +265,8 @@ func (c *Controller) updateStatus(healthy bool, msg, reason string, httpErr *ins
260265
}
261266

262267
// getPullSecret gets pull-secret as *v1.Secret
263-
func (c *Controller) getPullSecret() (*v1.Secret, error) {
264-
return c.coreClient.Secrets("openshift-config").Get(c.ctx, "pull-secret", metav1.GetOptions{})
268+
func (c *Controller) getPullSecret(ctx context.Context) (*v1.Secret, error) {
269+
return c.coreClient.Secrets("openshift-config").Get(ctx, "pull-secret", metav1.GetOptions{})
265270
}
266271

267272
// isUpdatedPullSecretContentSame checks if the updatedPS content is different

0 commit comments

Comments
 (0)