Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

Re-initialize project with operator-sdk and go.kubebuilder.io/v3 plugin #22

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

jianrongzhang89
Copy link
Contributor

No description provided.

@jianrongzhang89 jianrongzhang89 changed the title [WIP] Re-initialize project with operator-sdk and go.kubebuilder.io/v3 plugin Re-initialize project with operator-sdk and go.kubebuilder.io/v3 plugin Nov 16, 2023
@@ -1,20 +1,18 @@
# Adds namespace to all resources.
namespace: backstage-system
namespace: operator-system
Copy link
Member

@rm3l rm3l Nov 17, 2023

Choose a reason for hiding this comment

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

Can this be prefixed with backstage- as before? I noticed that the operator was installed into an operator-system namespace and the Deployment was named operator-controller-manager, which IMO might be confusing to people installing the operator.


# Value of this field is prepended to the
# names of all resources, e.g. a deployment named
# "wordpress" becomes "alices-wordpress".
# Note that it should also match with the prefix (text before '-') of the namespace
# field above.
namePrefix: backstage-
namePrefix: operator-
Copy link
Member

Choose a reason for hiding this comment

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

Same point as the previous comment.

Makefile Show resolved Hide resolved
@@ -1,71 +1,80 @@
module backstage.io/backstage-deploy-operator
module backstage.io/backstage-operator
Copy link
Member

Choose a reason for hiding this comment

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

I'm also wondering if the module name should not be changed to github.com/janus-idp/operator..

Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

Added a few more comments. But I guess all those comments can be resolved with the right operator-sdk init flags. I was able to get the intended result with the command below. WDYT?

$ operator-sdk init \
     --plugins go/v3 \
     --project-version 3 \
     --domain janus-idp.io \
     --repo github.com/janus-idp/operator \
     --project-name backstage-operator \
     --owner "Red Hat"

Copy link
Member

Choose a reason for hiding this comment

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

nit:

The Copyright notice currently shows: "Copyright 2023". I noticed operator-sdk init had a --owner flag to update that. So maybe we could use it: operator-sdk init [...] --owner "Red Hat"..

Makefile Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the old backstage operator (https://github.com/janus-idp/backstage-operator/blob/main/PROJECT), I think we could use the following values:

  • domain could be: janus-idp.io
  • projectName could be: backstage-operator
  • repo could be: github.com/janus-idp/operator

Copy link
Member

@gazarenkov gazarenkov left a comment

Choose a reason for hiding this comment

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

As Armal mentioned, there are several things in PROJECT (I do not think very important) and some in generated kustomize (more interesting but can be fixed later)
It works and I'd prefer to merge it sooner than later to not blocking everything else, so basically LGTM.

@rm3l
Copy link
Member

rm3l commented Nov 17, 2023

As Armal mentioned, there are several things in PROJECT (I do not think very important) and some in generated kustomize (more interesting but can be fixed later) It works and I'd prefer to merge it sooner than later to not blocking everything else, so basically LGTM.

👍🏿 Let's do that in subsequent issues/PRs later.

/lgtm

@jianrongzhang89 jianrongzhang89 merged commit 94bff62 into janus-idp:main Nov 17, 2023
1 check passed
@jianrongzhang89 jianrongzhang89 deleted the project-init branch November 17, 2023 13:02
@rm3l
Copy link
Member

rm3l commented Nov 17, 2023

As Armal mentioned, there are several things in PROJECT (I do not think very important) and some in generated kustomize (more interesting but can be fixed later) It works and I'd prefer to merge it sooner than later to not blocking everything else, so basically LGTM.

👍🏿 Let's do that in subsequent issues/PRs later.

To not forget about it: #23

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants