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.

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

adding a new target to deploy controller and Metadata server and crea… #222

Closed
wants to merge 9 commits into from

Conversation

mmorency2021
Copy link

@mmorency2021 mmorency2021 commented Sep 27, 2024

This is part of ORAN - Project infrastructure - Automatically start services
https://issues.redhat.com/browse/MGMT-18997

-using Target deploy to install operator and enable services

  • Using clusterID to deploy meta data server , user can edit it to match their cloudId

…ting new directory to put services manifest.
Copy link

openshift-ci bot commented Sep 27, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign donpenney for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

openshift-ci bot commented Sep 27, 2024

Hi @mmorency2021. Thanks for your PR.

I'm waiting for a openshift-kni member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 27, 2024
@donpenney donpenney requested review from Missxiaoguo, alegacy, bartwensley and irinamihai and removed request for jhernand and danielerez September 27, 2024 19:32
@donpenney
Copy link
Collaborator

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 27, 2024
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@@ -82,6 +82,10 @@ CONTAINER_TOOL ?= docker
SHELL = /usr/bin/env bash -o pipefail
.SHELLFLAGS = -ec

# setting the backend token and the backend URL
Copy link
Collaborator

Choose a reason for hiding this comment

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

None of these settings should be required. These can all be auto-detected by the operator at runtime.

Makefile Outdated
@@ -170,11 +174,13 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified
.PHONY: deploy
deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config.
cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG}
$(KUSTOMIZE) build config/default | $(KUBECTL) apply -f -
@sed -i '' -e 's/ingressHost:.*/ingressHost: $(INGRESS_HOST)/' config/services/metadata-server.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be possible to auto-detect this at runtime rather than specifying it.

Makefile Outdated
@@ -170,11 +174,13 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified
.PHONY: deploy
deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config.
cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG}
$(KUSTOMIZE) build config/default | $(KUBECTL) apply -f -
@sed -i '' -e 's/ingressHost:.*/ingressHost: $(INGRESS_HOST)/' config/services/metadata-server.yaml
@sed -i '' -e 's/cloudId: .*/cloudId: $(CLOUD_ID)/' config/services/metadata-server.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to clarify what this attribute was intended to be. If it is the "global" o-cloud id then it must come from the user rather than being hardcoded to something at deployment time.

cc: @irinamihai

Copy link
Author

Choose a reason for hiding this comment

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

actually i consider that the User should provide the cloud_id.
to deploy operator and service user should use below format

make deploy CLOUD_ID=c0332915-6bbb-4d8d-8628-0ab3cc2c7e5e INGRESS_HOST=o2ims.apps.hubcluster-1.hubcluster-1.lab.eng.cert.redhat.com

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, but in a real production environment this would not be deployed in single operation. First the operator would be deployed with a default InventoryCR to simply enable the services, and then later a user would have to edit that CR to add the global o-cloud id as well as other attributes (that are yet to be defined).

Copy link
Author

Choose a reason for hiding this comment

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

true. in production ofcourse Operator get deployed then user enabled services. this is why i n the first place i created two targets. one to deploy the operator and another one to enable the services.
waiting on @bartwensley comments since he created the Jira. thanks @alegacy.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I guess I wasn't clear... the deploying of the inventory CR should be in two steps... first the default version of it that enables the services without any extra config (e.g., o-cloud id)... and then later the user will come along and edit it to add configuration details to it (i.e., oauth credentials, SMO address, etc...)

Copy link
Author

Choose a reason for hiding this comment

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

using this , you can get the domain , but not the ingresshost.
oc get ingresscontrollers.operator.openshift.io -n openshift-ingress-operator default -o json | jq .status.domain
"apps.hubcluster-1.hubcluster-1.lab.eng.cert.redhat.com"

Copy link
Author

Choose a reason for hiding this comment

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

for the metadata server are you expecting the domain ? IngressHost is no tpart of the ingresscontrollers operator.

see below :

oc explain ingresscontrollers.operator.openshift.io.spec
GROUP: operator.openshift.io
KIND: IngressController
VERSION: v1

FIELD: spec

DESCRIPTION:
spec is the specification of the desired behavior of the IngressController.

FIELDS:
clientTLS
clientTLS specifies settings for requesting and verifying client
certificates, which can be used to enable mutual TLS for edge-terminated and
reencrypt routes.

defaultCertificate
defaultCertificate is a reference to a secret containing the default
certificate served by the ingress controller. When Routes don't specify
their own certificate, defaultCertificate is used.
The secret must contain the following keys and data:
tls.crt: certificate file contents tls.key: key file contents
If unset, a wildcard certificate is automatically generated and used. The
certificate is valid for the ingress controller domain (and subdomains) and
the generated certificate's CA will be automatically integrated with the
cluster's trust store.
If a wildcard certificate is used and shared by multiple HTTP/2 enabled
routes (which implies ALPN) then clients (i.e., notably browsers) are at
liberty to reuse open connections. This means a client can reuse a
connection to another route and that is likely to fail. This behaviour is
generally known as connection coalescing.
The in-use certificate (whether generated or user-specified) will be
automatically integrated with OpenShift's built-in OAuth server.

domain
domain is a DNS name serviced by the ingress controller and is used to
configure multiple features:
* For the LoadBalancerService endpoint publishing strategy, domain is used
to configure DNS records. See endpointPublishingStrategy.
* When using a generated default certificate, the certificate will be valid
for domain and its subdomains. See defaultCertificate.
* The value is published to individual Route statuses so that end-users
know where to target external DNS records.
domain must be unique among all IngressControllers, and cannot be updated.
If empty, defaults to ingress.config.openshift.io/cluster .spec.domain.

endpointPublishingStrategy
endpointPublishingStrategy is used to publish the ingress controller
endpoints to other networks, enable load balancer integrations, etc.
If unset, the default is based on
infrastructure.config.openshift.io/cluster .status.platform:
AWS: LoadBalancerService (with External scope) Azure:
LoadBalancerService (with External scope) GCP: LoadBalancerService
(with External scope) IBMCloud: LoadBalancerService (with External
scope) AlibabaCloud: LoadBalancerService (with External scope) Libvirt:
HostNetwork
Any other platform types (including None) default to HostNetwork.
endpointPublishingStrategy cannot be updated.

httpCompression
httpCompression defines a policy for HTTP traffic compression. By default,
there is no HTTP compression.

httpEmptyRequestsPolicy
httpEmptyRequestsPolicy describes how HTTP connections should be handled if
the connection times out before a request is received. Allowed values for
this field are "Respond" and "Ignore". If the field is set to "Respond",
the ingress controller sends an HTTP 400 or 408 response, logs the
connection (if access logging is enabled), and counts the connection in the
appropriate metrics. If the field is set to "Ignore", the ingress
controller closes the connection without sending a response, logging the
connection, or incrementing metrics. The default value is "Respond".
Typically, these connections come from load balancers' health probes or Web
browsers' speculative connections ("preconnect") and can be safely ignored.
However, these requests may also be caused by network errors, and so setting
this field to "Ignore" may impede detection and diagnosis of problems. In
addition, these requests may be caused by port scans, in which case logging
empty requests may aid in detecting intrusion attempts.

httpErrorCodePages
httpErrorCodePages specifies a configmap with custom error pages. The
administrator must create this configmap in the openshift-config namespace.
This configmap should have keys in the format "error-page-.http", where is an HTTP error code. For example,
"error-page-503.http" defines an error page for HTTP 503 responses.
Currently only error pages for 503 and 404 responses can be customized. Each
value in the configmap should be the full response, including HTTP headers.
Eg-
https://raw.githubusercontent.com/openshift/router/fadab45747a9b30cc3f0a4b41ad2871f95827a93/images/router/haproxy/conf/error-page-503.http
If this field is empty, the ingress controller uses the default error pages.

httpHeaders
httpHeaders defines policy for HTTP headers.
If this field is empty, the default values are used.

logging
logging defines parameters for what should be logged where. If this field
is empty, operational logs are enabled but access logs are disabled.

namespaceSelector
namespaceSelector is used to filter the set of namespaces serviced by the
ingress controller. This is useful for implementing shards.
If unset, the default is no filtering.

nodePlacement
nodePlacement enables explicit control over the scheduling of the ingress
controller.
If unset, defaults are used. See NodePlacement for more details.

replicas
replicas is the desired number of ingress controller replicas. If unset, the
default depends on the value of the defaultPlacement field in the cluster
config.openshift.io/v1/ingresses status.
The value of replicas is set based on the value of a chosen field in the
Infrastructure CR. If defaultPlacement is set to ControlPlane, the chosen
field will be controlPlaneTopology. If it is set to Workers the chosen field
will be infrastructureTopology. Replicas will then be set to 1 or 2 based
whether the chosen field's value is SingleReplica or HighlyAvailable,
respectively.
These defaults are subject to change.

routeAdmission
routeAdmission defines a policy for handling new route claims (for example,
to allow or deny claims across namespaces).
If empty, defaults will be applied. See specific routeAdmission fields for
details about their defaults.

routeSelector
routeSelector is used to filter the set of Routes serviced by the ingress
controller. This is useful for implementing shards.
If unset, the default is no filtering.

tlsSecurityProfile
tlsSecurityProfile specifies settings for TLS connections for
ingresscontrollers.
If unset, the default is based on the
apiservers.config.openshift.io/cluster resource.
Note that when using the Old, Intermediate, and Modern profile types, the
effective profile configuration is subject to change between releases. For
example, given a specification to use the Intermediate profile deployed on
release X.Y.Z, an upgrade to release X.Y.Z+1 may cause a new profile
configuration to be applied to the ingress controller, resulting in a
rollout.

tuningOptions
tuningOptions defines parameters for adjusting the performance of ingress
controller pods. All fields are optional and will use their respective
defaults if not set. See specific tuningOptions fields for more details.
Setting fields within tuningOptions is generally not recommended. The
default values are suitable for most configurations.

unsupportedConfigOverrides
unsupportedConfigOverrides allows specifying unsupported configuration
options. Its use is unsupported.

Copy link
Collaborator

@alegacy alegacy Sep 30, 2024

Choose a reason for hiding this comment

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

The ingress host is just the domain prepended with "apps.". This is the ingress controller domain on my system:

apps.hubcluster.hub.dev.vz.bos2.lab

and this is my ingress host:

o2ims.apps.hubcluster.hub.dev.vz.bos2.lab

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here's on the cloudIds from the O-RAN standard:

oCloudId: Identifier for the containing O-Cloud. This identifier is copied from the O-Cloud Id assigned by the SMO during the O-Cloud deployment.

globalcloudId: Identifier of the O-Cloud instance. Globally unique across O-Cloud instances. This value was provided by the SMO with the callback URI at the beginning of O-Cloud genesis and used in registration at the end of genesis.

On the ingressHost: I agree with @alegacy that it can be derived, as he pointed out, from the ingresscontroller or from one of the existing routes. The user could then go ahead and edit it if desired.

Copy link
Author

Choose a reason for hiding this comment

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

on our side we can deploy the operator with the service using the domain , then user can edit it after? in my lab i just add a prefix to the domain to set the ingressHost.
i will do the changes according to your comments. thanks all.

Makefile Outdated Show resolved Hide resolved
deleting the ressource file since , in this PR we are only deploying the metadata server. based on comments we will adjust and add resource server deployment as well.
Makefile Show resolved Hide resolved
@bartwensley
Copy link
Collaborator

This is part of ORAN - Project infrastructure - Automatically start services https://issues.redhat.com/browse/MGMT-18997

  • Adding new target called: deploy-service to deploy IMS operator controller and metadata server. other services like ressource and deployment server will be added later.

I would like this to be done through the existing "make deploy" - deploying the operator should also start the services. We don't need a target to deploy the operator without starting the services.

Comment on lines 1 to 10
apiVersion: apps/v1
kind: Deployment
metadata:
name: controller-manager
namespace: system
spec:
template:
spec:
containers:
- name: manager
Copy link
Collaborator

@pixelsoccupied pixelsoccupied Sep 30, 2024

Choose a reason for hiding this comment

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

I'm curious why we are not generating this at runtime from the operator and then applying it with client-go/controller-runtime? See an example https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v4/memcached-operator/internal/controller/memcached_controller.go#L306

This we can capture all the tokens/hosts/etc and construct the required CRs at runtime and once deployed we can monitoring this at every requeue

From a dev/deploy we simply make deploy and everything is handled automatically. In other words, we deploy the operator and the operator deploys and manages everything else for us

@donpenney
Copy link
Collaborator

Please note that this update impacts the generated manifests, which must also be included in the PR

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 2, 2024
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

namespace: oran-o2ims
spec:
alarmSubscriptionServerConfig:
enabled: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does enabled mean in this context ?

Copy link
Author

Choose a reason for hiding this comment

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

Enabled indicates if the alarm server should be started or not.

adding env-config.yaml in Kustomization file.
Copy link

openshift-ci bot commented Oct 7, 2024

@mmorency2021: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ci-job f03abfd link true /test ci-job
ci/prow/ci-bundle-operator-bundle f03abfd link true /test ci-bundle-operator-bundle
ci/prow/images f03abfd link true /test images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@mmorency2021
Copy link
Author

i had to close this PR since they have changed some logic upstream. i created a target called deploy but a target called deploy has been pushed to main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants