-
Notifications
You must be signed in to change notification settings - Fork 66
🌱 Move Registry YAML and remove use of namespace in kustomizations #2088
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.
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
Closed
+290
−113
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,4 +12,4 @@ roleRef: | |
subjects: | ||
- kind: ServiceAccount | ||
name: controller-manager | ||
namespace: system | ||
namespace: olmv1-system |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,4 +9,4 @@ roleRef: | |
subjects: | ||
- kind: ServiceAccount | ||
name: controller-manager | ||
namespace: system | ||
namespace: olmv1-system |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,4 @@ apiVersion: v1 | |
kind: ServiceAccount | ||
metadata: | ||
name: controller-manager | ||
namespace: system | ||
namespace: olmv1-system |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 0 additions & 1 deletion
1
config/components/cert-manager/operator-controller/kustomization.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,4 @@ apiVersion: kustomize.config.k8s.io/v1alpha1 | |
kind: Component | ||
components: | ||
- coverage | ||
- registries-conf | ||
- registry |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
apiVersion: cert-manager.io/v1 | ||
kind: Certificate | ||
metadata: | ||
name: operator-controller-e2e-registry | ||
namespace: operator-controller-e2e | ||
spec: | ||
secretName: operator-controller-e2e-registry | ||
isCA: true | ||
dnsNames: | ||
- docker-registry.operator-controller-e2e.svc | ||
- docker-registry.operator-controller-e2e.svc.cluster.local | ||
- docker-registry-controller-manager-metrics-service.operator-controller-e2e.svc | ||
- docker-registry-controller-manager-metrics-service.operator-controller-e2e.svc.cluster.local | ||
privateKey: | ||
algorithm: ECDSA | ||
size: 256 | ||
issuerRef: | ||
name: olmv1-ca | ||
kind: ClusterIssuer | ||
group: cert-manager.io |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
metadata: | ||
name: docker-registry | ||
namespace: operator-controller-e2e | ||
labels: | ||
app: registry | ||
spec: | ||
replicas: 1 | ||
selector: | ||
matchLabels: | ||
app: registry | ||
template: | ||
metadata: | ||
labels: | ||
app: registry | ||
spec: | ||
containers: | ||
- name: registry | ||
image: registry:3 | ||
imagePullPolicy: IfNotPresent | ||
volumeMounts: | ||
- name: certs-vol | ||
mountPath: "/certs" | ||
env: | ||
- name: REGISTRY_HTTP_ADDR | ||
value: ":5000" | ||
- name: REGISTRY_HTTP_TLS_CERTIFICATE | ||
value: "/certs/tls.crt" | ||
- name: REGISTRY_HTTP_TLS_KEY | ||
value: "/certs/tls.key" | ||
volumes: | ||
- name: certs-vol | ||
secret: | ||
secretName: operator-controller-e2e-registry |
7 changes: 5 additions & 2 deletions
7
...ts/e2e/registries-conf/kustomization.yaml → ...omponents/e2e/registry/kustomization.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
apiVersion: kustomize.config.k8s.io/v1alpha1 | ||
kind: Component | ||
namespace: olmv1-system | ||
resources: | ||
- registries_conf_configmap.yaml | ||
- certificate.yaml | ||
- deployment.yaml | ||
- configmap.yaml | ||
- service.yaml | ||
- namespace.yaml | ||
patches: | ||
- path: manager_e2e_registries_conf_patch.yaml |
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
apiVersion: v1 | ||
kind: Namespace | ||
metadata: | ||
name: operator-controller-e2e |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
apiVersion: v1 | ||
kind: Service | ||
metadata: | ||
name: docker-registry | ||
namespace: operator-controller-e2e | ||
spec: | ||
selector: | ||
app: registry | ||
ports: | ||
- name: http | ||
port: 5000 | ||
targetPort: 5000 | ||
nodePort: 30000 | ||
type: NodePort |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if we need to change the name, do we have to update all the files after this change?
Also, I haven't checked, but in the downstream setup, are we using a shell script to make the change, or are we properly setting the Kustomize value and then building?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we not do the other changes and keep the namespace definition on the kustomize?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is making the kustomization more robust.
Downstream, the kustomization is done with a completely different overlay, and thus, sets the namespace appropriately according to openshift.
It's non-trivial to change the namespace via kustomize; there are also odd interdependencies based on ordering of components that may cause the namespaces of components outside of
olmv1-system
to be overwritten. For example, the CA components need(ed/s) to be last, or theircert-manager
namespace will be overwritten witholmv1-system
. Downstream doesn't have to deal with cert-manager, so there's no issues with namespace overwriting.This change is something @joelanford had been looking at, in his reorganization of kustomize (tagging him as he has some interest here).
We never really gave people an easy way to change the namespace anyways, as the installation was done via the quickstart-generated files, and those don't have options to change the namespace.
If we want people to change the namespace, perhaps we should add functionality to the resulting
install.sh
script?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my only concern in the PR. That said, if everyone is aligned with the change, I’m okay moving forward with it.
If we want to provide this flexibility, we’ll need to replace the namespace (ns) in all places eventually—regardless of whether we use Kustomize to build it or not.
The only downside I see is that this information isn't currently centralised, so we’ll need to remember to update every reference manually. That said, a simple find-and-replace should be enough to handle it if changes are needed later. We can either add validation to ensure/ci if we wish to do so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There has been some discussion on moving the configuration from kustomize to helm... but that's a project in-and-of itself...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having problems with the upgrade-e2e, at this point, I want to split this into two PRs, the namespace change and the registry change.