Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promote ConsolePlugin API version to v1 #440

Merged
merged 18 commits into from
May 26, 2023
Merged

Promote ConsolePlugin API version to v1 #440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
efa6ce5
Promote ConsolePlugin API version to v1
ciiay Feb 15, 2023
21519fb
Merge branch 'master' of https://github.com/redhat-developer/gitops-o…
ciiay Feb 21, 2023
849f07d
Merge branch 'master' of https://github.com/redhat-developer/gitops-o…
ciiay Feb 23, 2023
362746c
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
varshab1210 Mar 4, 2023
dd5c59b
Merge branch 'master' of https://github.com/redhat-developer/gitops-o…
ciiay Mar 9, 2023
4b81520
Merge branch 'gitops-2646-update-dynamic-plugin-cr' of https://github…
ciiay Mar 9, 2023
e6b8824
Merge branch 'master' of https://github.com/redhat-developer/gitops-o…
ciiay Mar 28, 2023
f357465
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
varshab1210 Mar 28, 2023
b0de33c
Merge branch 'master' of https://github.com/redhat-developer/gitops-o…
ciiay Mar 29, 2023
c9593cb
Merge branch 'gitops-2646-update-dynamic-plugin-cr' of https://github…
ciiay Mar 29, 2023
c5e8ab0
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
varshab1210 Apr 20, 2023
214c555
Merge branch 'master' of https://github.com/redhat-developer/gitops-o…
ciiay Apr 21, 2023
fbe4697
Merge branch 'gitops-2646-update-dynamic-plugin-cr' of https://github…
ciiay Apr 21, 2023
f2b9a66
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
varshab1210 Apr 25, 2023
7c11c9e
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
varshab1210 Apr 26, 2023
f6ea081
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
varshab1210 Apr 27, 2023
41ae850
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
ciiay Apr 28, 2023
efb5627
Merge branch 'master' into gitops-2646-update-dynamic-plugin-cr
ciiay May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions controllers/consoleplugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"os"
"reflect"

consolepluginv1 "github.com/openshift/api/console/v1alpha1"
consolev1 "github.com/openshift/api/console/v1"
pipelinesv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1"
"github.com/redhat-developer/gitops-operator/controllers/util"

Expand Down Expand Up @@ -159,18 +159,24 @@ func pluginDeployment() *appsv1.Deployment {
}
}

func consolePlugin() *consolepluginv1.ConsolePlugin {
return &consolepluginv1.ConsolePlugin{
func consolePlugin() *consolev1.ConsolePlugin {
return &consolev1.ConsolePlugin{
ObjectMeta: metav1.ObjectMeta{
Name: gitopsPluginName,
},
Spec: consolepluginv1.ConsolePluginSpec{
Spec: consolev1.ConsolePluginSpec{
DisplayName: displayName,
Service: consolepluginv1.ConsolePluginService{
Name: gitopsPluginName,
Namespace: serviceNamespace,
Port: servicePort,
BasePath: "/",
Backend: consolev1.ConsolePluginBackend{
Type: consolev1.Service,
Service: &consolev1.ConsolePluginService{
Name: gitopsPluginName,
Namespace: serviceNamespace,
Port: servicePort,
BasePath: "/",
},
},
I18n: consolev1.ConsolePluginI18n{
LoadType: consolev1.Preload,
},
},
}
Expand Down Expand Up @@ -332,7 +338,7 @@ func (r *ReconcileGitopsService) reconcileConsolePlugin(instance *pipelinesv1alp
return reconcile.Result{}, err
}

existingPlugin := &consolepluginv1.ConsolePlugin{}
existingPlugin := &consolev1.ConsolePlugin{}

if err := r.Client.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName},
existingPlugin); err != nil {
Expand All @@ -349,12 +355,12 @@ func (r *ReconcileGitopsService) reconcileConsolePlugin(instance *pipelinesv1alp
}
} else {
changed := !reflect.DeepEqual(existingPlugin.Spec.DisplayName, newConsolePlugin.Spec.DisplayName) ||
!reflect.DeepEqual(existingPlugin.Spec.Service, newConsolePlugin.Spec.Service)
!reflect.DeepEqual(existingPlugin.Spec.Backend.Service, newConsolePlugin.Spec.Backend.Service)

if changed {
reqLogger.Info("Reconciling Console Plugin", "Namespace", existingPlugin.Namespace, "Name", existingPlugin.Name)
existingPlugin.Spec.DisplayName = newConsolePlugin.Spec.DisplayName
existingPlugin.Spec.Service = newConsolePlugin.Spec.Service
existingPlugin.Spec.Backend.Service = newConsolePlugin.Spec.Backend.Service
return reconcile.Result{}, r.Client.Update(context.TODO(), newConsolePlugin)
}
}
Expand Down
54 changes: 31 additions & 23 deletions controllers/consoleplugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"testing"

consolepluginv1 "github.com/openshift/api/console/v1alpha1"
consolev1 "github.com/openshift/api/console/v1"
pipelinesv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1"
"gotest.tools/assert"
appsv1 "k8s.io/api/apps/v1"
Expand All @@ -25,13 +25,15 @@ func TestPlugin(t *testing.T) {
testDisplayName := displayName
assert.Equal(t, testConsolePlugin.Spec.DisplayName, testDisplayName)

testPluginService := consolepluginv1.ConsolePluginService{
testPluginService := &consolev1.ConsolePluginService{
Name: gitopsPluginName,
Namespace: serviceNamespace,
Port: servicePort,
BasePath: "/",
}
assert.DeepEqual(t, testConsolePlugin.Spec.Service, testPluginService)
assert.DeepEqual(t, testConsolePlugin.Spec.I18n.LoadType, consolev1.Preload)
assert.DeepEqual(t, testConsolePlugin.Spec.Backend.Type, consolev1.Service)
assert.DeepEqual(t, testConsolePlugin.Spec.Backend.Service, testPluginService)
}

func TestPlugin_reconcileDeployment_changedLabels(t *testing.T) {
Expand Down Expand Up @@ -1388,7 +1390,7 @@ func TestPlugin_reconcileConsolePlugin(t *testing.T) {
_, err := reconciler.reconcileConsolePlugin(instance, newRequest(serviceNamespace, gitopsPluginName))
assertNoError(t, err)

consolePlugin := &consolepluginv1.ConsolePlugin{}
consolePlugin := &consolev1.ConsolePlugin{}
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName}, consolePlugin)
assertNoError(t, err)
}
Expand Down Expand Up @@ -1417,17 +1419,20 @@ func TestPlugin_reconcileConsolePlugin_changedDisplayName(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cp := &consolepluginv1.ConsolePlugin{
cp := &consolev1.ConsolePlugin{
ObjectMeta: metav1.ObjectMeta{
Name: gitopsPluginName,
},
Spec: consolepluginv1.ConsolePluginSpec{
Spec: consolev1.ConsolePluginSpec{
DisplayName: test.displayName,
Service: consolepluginv1.ConsolePluginService{
Name: gitopsPluginName,
Namespace: serviceNamespace,
Port: servicePort,
BasePath: "/",
Backend: consolev1.ConsolePluginBackend{
Type: consolev1.Service,
Service: &consolev1.ConsolePluginService{
Name: gitopsPluginName,
Namespace: serviceNamespace,
Port: servicePort,
BasePath: "/",
},
},
},
}
Expand All @@ -1436,7 +1441,7 @@ func TestPlugin_reconcileConsolePlugin_changedDisplayName(t *testing.T) {
_, err := reconciler.reconcileConsolePlugin(instance, newRequest(serviceNamespace, gitopsPluginName))
assertNoError(t, err)

consolePlugin := &consolepluginv1.ConsolePlugin{}
consolePlugin := &consolev1.ConsolePlugin{}
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName}, consolePlugin)
assertNoError(t, err)

Expand All @@ -1447,11 +1452,11 @@ func TestPlugin_reconcileConsolePlugin_changedDisplayName(t *testing.T) {
func TestPlugin_reconcileConsolePlugin_changedService(t *testing.T) {
tests := []struct {
name string
service consolepluginv1.ConsolePluginService
service consolev1.ConsolePluginService
}{
{
name: "default service",
service: consolepluginv1.ConsolePluginService{
service: consolev1.ConsolePluginService{
Name: gitopsPluginName,
Namespace: serviceNamespace,
Port: servicePort,
Expand All @@ -1460,7 +1465,7 @@ func TestPlugin_reconcileConsolePlugin_changedService(t *testing.T) {
},
{
name: "changed service",
service: consolepluginv1.ConsolePluginService{
service: consolev1.ConsolePluginService{
Name: "wrong name",
Namespace: "wrong namespace",
Port: int32(9002),
Expand All @@ -1478,28 +1483,31 @@ func TestPlugin_reconcileConsolePlugin_changedService(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cp := &consolepluginv1.ConsolePlugin{
cp := &consolev1.ConsolePlugin{
ObjectMeta: metav1.ObjectMeta{
Name: gitopsPluginName,
},
Spec: consolepluginv1.ConsolePluginSpec{
Spec: consolev1.ConsolePluginSpec{
DisplayName: displayName,
Service: test.service,
Backend: consolev1.ConsolePluginBackend{
Type: consolev1.Service,
Service: &test.service,
},
},
}
reconciler.Client.Create(context.TODO(), cp)

_, err := reconciler.reconcileConsolePlugin(instance, newRequest(serviceNamespace, gitopsPluginName))
assertNoError(t, err)

consolePlugin := &consolepluginv1.ConsolePlugin{}
consolePlugin := &consolev1.ConsolePlugin{}
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName}, consolePlugin)
assertNoError(t, err)

assert.Equal(t, consolePlugin.Spec.Service.Name, "gitops-plugin")
assert.Equal(t, consolePlugin.Spec.Service.Namespace, "openshift-gitops")
assert.Equal(t, consolePlugin.Spec.Service.Port, int32(9001))
assert.Equal(t, consolePlugin.Spec.Service.BasePath, "/")
assert.Equal(t, consolePlugin.Spec.Backend.Service.Name, "gitops-plugin")
assert.Equal(t, consolePlugin.Spec.Backend.Service.Namespace, "openshift-gitops")
assert.Equal(t, consolePlugin.Spec.Backend.Service.Port, int32(9001))
assert.Equal(t, consolePlugin.Spec.Backend.Service.BasePath, "/")
})
}
}
Expand Down
11 changes: 5 additions & 6 deletions controllers/gitopsservice_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
configv1 "github.com/openshift/api/config/v1"
consolev1 "github.com/openshift/api/console/v1"
consolepluginv1 "github.com/openshift/api/console/v1alpha1"
routev1 "github.com/openshift/api/route/v1"
pipelinesv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1"
"github.com/redhat-developer/gitops-operator/common"
Expand Down Expand Up @@ -245,10 +244,10 @@ func TestReconcile(t *testing.T) {
assert.DeepEqual(t, deploy.Spec.Template.Spec.Containers[0].Image, backendImage)

// Check if plugin instance is created in openshift-gitops namespace
consolePlugin := &consolepluginv1.ConsolePlugin{}
consolePlugin := &consolev1.ConsolePlugin{}
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName}, consolePlugin)
assertNoError(t, err)
assert.DeepEqual(t, consolePlugin.Spec.Service.Name, gitopsPluginName)
assert.DeepEqual(t, consolePlugin.Spec.Backend.Service.Name, gitopsPluginName)

pluginDeploy := &appsv1.Deployment{}
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, pluginDeploy)
Expand Down Expand Up @@ -310,7 +309,7 @@ func TestReconcile_consoleAPINotFound(t *testing.T) {
assertNoError(t, err)

// Check consolePlugin and other resources are not created
consolePlugin := &consolepluginv1.ConsolePlugin{}
consolePlugin := &consolev1.ConsolePlugin{}
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName}, consolePlugin)
assert.Error(t, err, "consoleplugins.console.openshift.io \"gitops-plugin\" not found")

Expand Down Expand Up @@ -342,7 +341,7 @@ func TestReconcile_ocpVersionLowerThan4_15(t *testing.T) {
assertNoError(t, err)

// Check consolePlugin and other resources are not created
consolePlugin := &consolepluginv1.ConsolePlugin{}
consolePlugin := &consolev1.ConsolePlugin{}
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName}, consolePlugin)
assert.Error(t, err, "consoleplugins.console.openshift.io \"gitops-plugin\" not found")

Expand Down Expand Up @@ -651,7 +650,7 @@ func addKnownTypesToScheme(scheme *runtime.Scheme) {
scheme.AddKnownTypes(argoapp.GroupVersion, &argoapp.ArgoCD{})
scheme.AddKnownTypes(consolev1.GroupVersion, &consolev1.ConsoleCLIDownload{})
scheme.AddKnownTypes(routev1.GroupVersion, &routev1.Route{})
scheme.AddKnownTypes(consolepluginv1.GroupVersion, &consolepluginv1.ConsolePlugin{})
scheme.AddKnownTypes(consolev1.GroupVersion, &consolev1.ConsolePlugin{})
}

func newReconcileGitOpsService(client client.Client, scheme *runtime.Scheme) *ReconcileGitopsService {
Expand Down
2 changes: 0 additions & 2 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
appsv1 "github.com/openshift/api/apps/v1"
configv1 "github.com/openshift/api/config/v1"
console "github.com/openshift/api/console/v1"
consolepluginv1 "github.com/openshift/api/console/v1alpha1"
oauthv1 "github.com/openshift/api/oauth/v1"
routev1 "github.com/openshift/api/route/v1"
templatev1 "github.com/openshift/api/template/v1"
Expand Down Expand Up @@ -113,7 +112,6 @@ func main() {
registerComponentOrExit(mgr, operatorsv1alpha1.AddToScheme)
registerComponentOrExit(mgr, argoapi.AddToScheme)
registerComponentOrExit(mgr, configv1.AddToScheme)
registerComponentOrExit(mgr, consolepluginv1.AddToScheme)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ciiay looks like we accidentally removed the line instead of updating it
if this line is not present the gitops operator will not be able to create the console plugin deployment at all

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaideepr97 Thanks for the review. This line was removed purposely. Because after updating to v1, the console plugin API is the same as console API, see Line 40, 41. And the console.AddToScheme is already there in L 109
registerComponentOrExit(mgr, console.AddToScheme)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, interesting that we had both to begin with
thanks for clearing that up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaideepr97 Because when we started console plugin the plugin SDK feature was only on the console/v1alpha1 version, then later they updated it and the new feature in v1alpha1 got merged into the console/v1.

registerComponentOrExit(mgr, monitoringv1.AddToScheme)
registerComponentOrExit(mgr, rolloutManagerApi.AddToScheme)
registerComponentOrExit(mgr, templatev1.AddToScheme)
Expand Down