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

Add oVirt platform status #451

Merged
merged 1 commit into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions config/v1/types_infrastructure.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ type PlatformStatus struct {
// OpenStack contains settings specific to the OpenStack infrastructure provider.
// +optional
OpenStack *OpenStackPlatformStatus `json:"openstack,omitempty"`

// Ovirt contains settings specific to the oVirt infrastructure provider.
// +optional
Copy link
Contributor

Choose a reason for hiding this comment

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

@damemi one needs the kubebuilder optional tag as well, doesn't one?

Copy link
Contributor

Choose a reason for hiding this comment

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

though it does not matter here as the default is optional already

Ovirt *OvirtPlatformStatus `json:"ovirt,omitempty"`
}

// AWSPlatformStatus holds the current status of the Amazon Web Services infrastructure provider.
Expand Down Expand Up @@ -199,6 +203,26 @@ type OpenStackPlatformStatus struct {
NodeDNSIP string `json:"nodeDNSIP,omitempty"`
}

// OvirtPlatformStatus holds the current status of the oVirt infrastructure provider.
type OvirtPlatformStatus struct {
// apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used
// by components inside the cluster, like kubelets using the infrastructure rather
// than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI
// points to. It is the IP for a self-hosted load balancer in front of the API servers.
APIServerInternalIP string `json:"apiServerInternalIP,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

so these fields are required but omitempty ??

@sttts / @deads2k any guidance here?

Copy link

Choose a reason for hiding this comment

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

omitempty with string will cause they are not printed when empty, but it doesn't make any difference during parsing. I don't think we have clear guidance for it, though.

Copy link
Contributor

@sttts sttts Oct 10, 2019

Choose a reason for hiding this comment

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

omitempty for optional fields is often a good idea. Usually only if you want to distinguish undefined and zero value, you don't use omitempty. Though then these fields are also pointers to survive Golang roundtripping.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/openshift/api/blob/master/config/v1/doc.go#L5

all fields are kindof optional at package level, and this is similar to other structs, so following that seems fine to me.


// ingressIP is an external IP which routes to the default ingress controller.
// The IP is a suitable target of a wildcard DNS record used to resolve default route host names.
IngressIP string `json:"ingressIP,omitempty"`

// nodeDNSIP is the IP address for the internal DNS used by the
// nodes. Unlike the one managed by the DNS operator, `NodeDNSIP`
// provides name resolution for the nodes themselves. There is no DNS-as-a-service for
// oVirt deployments. In order to minimize necessary changes to the
// datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames
// to the nodes in the cluster.
NodeDNSIP string `json:"nodeDNSIP,omitempty"`
}
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// InfrastructureList is
Expand Down
21 changes: 21 additions & 0 deletions config/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions config/v1/zz_generated.swagger_doc_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.