-
Notifications
You must be signed in to change notification settings - Fork 352
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
Add label to service #23
Conversation
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.
Thanks @dmvolod, just few minor notes.
README.md
Outdated
@@ -46,6 +46,12 @@ kamel install | |||
|
|||
This will configure the cluster with the Camel K custom resource definitions and install the operator on the current namespace. | |||
|
|||
If requires to uninstall Camel K from the OpenShift or Kubernetes, it's nessesary to run following command using "oc" or "kubectl" tool: |
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.
Maybe better moving this to the bottom, in a specific paragraph about uninstallation
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.
Moved it.
@@ -0,0 +1,19 @@ | |||
apiVersion: v1 |
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.
Ok, is this required for the metrics feature of the operator-sdk? I don't recall why it's missing
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 mean the service
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.
Yes, this is for metrics. It creates by default together with deployment but it's unable to control it parameters there. This is reason why we create it separately.
@@ -86,7 +86,6 @@ spec: | |||
metadata: | |||
labels: | |||
name: camel-k-operator | |||
app: "camel-k" |
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.
Maybe this is out-of-sync. I've added a make build-embed-resources
to sync that file. We should keep "camel-k" in the app label for the infrastructure components.
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.
Found it. It's the operator sdk that changes it. We can leave it and the source file without the label, as the file is not used (for this reason).
793c64d
to
05cf0ab
Compare
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.
Thanks!
* add local repository flag
Missing to add label to service. This requires to separate service creation in single file operator-service.yaml.