CNV-2705 Creating and exposing a VM-backed service

[bgaydosrh]

Label enterprise-4.5 and enterprise-4.6.

Test Build: https://cnv-2705--ocpdocs.netlify.app/openshift-enterprise/latest/virt/virtual_machines/vm_networking/virt-using-the-default-pod-network-with-virt.html#virt-creating-a-service-from-a-virtual-machine_virt-using-the-default-pod-network-with-virt

Tagging @phoracek for code review.

@phoracek , four questions:

1. Are there commands to start and connect to the VirtualMachineInstance that you can supply? I feel they should be at the start of the procedure (before current step Tweaked formatting on README #1).

2. For this Note: Labels on a VirtualMachineInstance are passed through to the pod. The labels on the VirtualMachineInstance must match the labels on the Service object. --- is this referring to the special: key: label in both VMI and Service YAML? If so I can highlight that with a callout.

3. In the U/S doc there is another approach (using virtctl) to expose the services. A general rule in doc is to give the user the one, best, most consistently successful way of doing a task. Is it your opinion we can leave the virtctl method out of this procedure? We could also put a link to the upstream doc in the assembly but that might lead to confusion as to what doc to follow. I'm open to any ideas or suggestions as I think you have a better grasp of the user audience here. I can of course add a note about using virtctl as well.

4. I am assuming the user can select any service name they like, unlike type which seems to have predefined valid values. Let me know if this is correct. I tried to emphasize that the service name should be descriptive to the service being created and exposed.

[openshift-docs-preview-bot]

The preview will be available shortly at:

• https://CNV-2705--ocpdocs.netlify.com/

[maiqueb]

Just some suggestions.

[bgaydosrh]

Thanks everyone for your comments and also chatted with @phoracek. I have a good idea now of what needs to change (connecting via name rather than type is a great idea, BTW, and will simplify things a lot). I'll have a another draft available for review this week.

Bob

[rhrazdil]

Looking good, I just think there is an extra colon in couple places

[bgaydosrh]

@phoracek --- I still have the question about whether we include a namespace attribute in the service YAML in response to the comment from @maiqueb above. The code is currently:

`apiVersion: v1
kind: Service
metadata:
name: vmservice
spec:
ports:

• port: 27017
  protocol: TCP
  targetPort: 22
  selector:
  special: key
  type: ClusterIP`

I added this statement:

Service is scoped by namespace. Ensure you reference the correct namespace
attribute when creating the Service YAML.

Is this statement true as I have worded it, and if so, should there not be a namespace attribute in the service YAML and what namespace should we supply? Or is this statement OK with no example of namespace attribute? If we include an attribute I can add a call-out.

[yossisegev]

Is this statement true as I have worded it

Yes, It is.

should there not be a namespace attribute in the service YAML

It depends. If the user creates the both VM and the service when the cluster context is focused on the same namespace (for example - he ran oc project <some-namespace>, and created the VM and the service), then they both will be created on the same namespace (unless any of them has a "namespace" key in its spec).
Personally, I believe its better practice to explicitly specify the destination namespace in the resource (any namespaced resource) spec.

and what namespace should we supply?

Whatever namespace the user want, as long as it exists on the cluster (and is not restricted by some permissions issue and such).

Or is this statement OK with no example of namespace attribute?

As I mentioned above - it depends.
Anyway - the place to put the namespace is under metadata (next to metadata.name).

[bergerhoffer]

A few more things.

[bgaydosrh]

@bergerhoffer and @sjhala-ccs - I tried using the .Verification steps after looking into it and think it's pretty cool. Let me know if I'm not doing something right, as it's my first time out with this feature.

[bgaydosrh]

Tagging @yossisegev for a final recheck on this topic as it's had substantial revision from peer review. Wondering specifically about recommendation to use oc instead of kubectl. If this LG I will send to @bergerhoffer for final check and merge.

See: https://cnv-2705--ocpdocs.netlify.app/openshift-enterprise/latest/virt/virtual_machines/vm_networking/virt-using-the-default-pod-network-with-virt.html#virt-creating-a-service-from-a-virtual-machine_virt-using-the-default-pod-network-with-virt

[yossisegev]

Some minor fixes, but other than that - the documentation is accurate (including the examples, which I ran and they work perfectly).

[yossisegev]

LGTM
Cool!

[adellape]

Looking good, just a few nits/questions left.

[adellape]

s/object./object:/

[bgaydosrh]

Fixed. Still kind of confused about when to use colons as I have been told different things by other reviewers. What I have done in the past is only use them if the sentence says something like "Edit the service YAML as follows" (a direct reference). A lot of editors I've worked with in the pas are just allergic to colons in general, I think. I have no specific preference, just trying to get a handle on what is expected... :-)

[adellape]

Gotcha. Yeah I noticed the difference with the "as follows:" earlier. I've typically just always had lead-in colons for any command/action we're telling them to do directly following. Worth bringing up to get clarity between reviewers/the team, though, for sure.

[adellape]

s/service./service:/

[bgaydosrh]

Fixed.

[adellape]

s/machine./machine:/

[bgaydosrh]

Fixed.

[adellape]

s/YAML./YAML:/

[bgaydosrh]

Fixed.

[adellape]

It looks like you're mostly using the updated API object formatting guidelines throughout, so I would expect something like "Service object" to have been uppercased+backticked:

`Service` object

Same question for some of the "Service YAML" instances throughout. Seems like those could be uppercase+backtick, or I could possibly see an argument for lowercase+plaintext if that was to be considered a general reference, but uppercase+plaintext seems like it's caught in the middle.

[bgaydosrh]

Yeah... again, it's a case of being told something else by other reviewers. My thinking has been to just been to uppercase object name as API uses it. But since we ref the YAML it does make sense to tick it. Same for "Service YAML" as you say.

So I changed all instances of Service to Service for Service object and Service YAML.

[adellape]

It seems strange to have a "Vertification steps" title in the middle of the procedure (though I realize the usage here is about showing the verification steps of this specific step). I would have expected these only to appear after a completed procedure. At least it seems like that was the intended usage per https://redhat-documentation.github.io/modular-docs/#procedure-module-guidelines.

I think the command that follows here would work just as well without the "Verification steps" title in the middle.

[bgaydosrh]

Ignoring for now as we discussed.

[adellape]

This should have an anchor/ID added to it. Also since this is within a module, it should be .Additional resources, as using == Additional resource should be saved for the assembly level (see the Note box in Writing assemblies).

[bgaydosrh]

In assembly, changed to:

include::modules/virt-creating-a-service-from-a-virtual-machine.adoc[leveloffset=+1]
.Additional resources

• xref:../../../networking/configuring_ingress_cluster_traffic/configuring-externalip.adoc#configuring-externalip[Configuring external IPs]

[adellape]

Latest changes LGTM!

[adellape]

/cherrypick enterprise-4.5

[adellape]

/cherrypick enterprise-4.6

[openshift-cherrypick-robot]

@adellape: new pull request created: #27128

In response to this:

/cherrypick enterprise-4.5

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/test-infra repository.

[openshift-cherrypick-robot]

@adellape: new pull request created: #27129

In response to this:

/cherrypick enterprise-4.6

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/test-infra repository.

[aburdenthehand]

@bgaydosrh Similar to #26676, I see that this was not cherry-picked to enterprise-4.7 and it's creating merge conflicts (#27562).
I can't see how the changes in this PR don't also apply to enterprise-4.7. Since this PR was initially created before enterprise-4.7 was created, I'm going to presume that it was an oversight and this PR should have been updated to also be cherry-picked to enterprise-4.7, which I'll do now so that it's in line with master and e-4.6.
If this is in error, I'll take care of undoing it.

@vikram-redhat in case you have more context

[aburdenthehand]

/cherrypick enterprise-4.7

[openshift-cherrypick-robot]

@aburdenthehand: new pull request created: #27680

In response to this:

/cherrypick enterprise-4.7

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/test-infra repository.