Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Rename .spec.image.ociClaim.ref to .spec.image.oci for simplicity #311

Merged
merged 6 commits into from
Aug 12, 2019
Merged

Rename .spec.image.ociClaim.ref to .spec.image.oci for simplicity #311

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
81cf9cd
OCIImageClaim -> OCIRef, simplify by naming the field `oci`
twelho Aug 9, 2019
f023830
Add conversion for `v1alpha1` for OCIClaim <-> OCIRef
twelho Aug 9, 2019
c8b42d8
Update reconciliation loop to use OCIRef
twelho Aug 9, 2019
0c7eb4a
make autogen tidy
twelho Aug 9, 2019
01e0850
Fix small typo
twelho Aug 9, 2019
9d6ab9e
Address feedback, OCIRef -> OCI, remove extras, consolidate conversion
twelho Aug 12, 2019
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
2 changes: 1 addition & 1 deletion cmd/ignite/cmd/vmcmd/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func addCreateFlags(fs *pflag.FlagSet, cf *run.CreateFlags) {
// Register more complex flags with their own flag types
cmdutil.SizeVar(fs, &cf.VM.Spec.Memory, "memory", "Amount of RAM to allocate for the VM")
cmdutil.SizeVarP(fs, &cf.VM.Spec.DiskSize, "size", "s", "VM filesystem size, for example 5GB or 2048MB")
cmdutil.OCIImageRefVarP(fs, &cf.VM.Spec.Kernel.OCIClaim.Ref, "kernel-image", "k", "Specify an OCI image containing the kernel at /boot/vmlinux and optionally, modules")
cmdutil.OCIImageRefVarP(fs, &cf.VM.Spec.Kernel.OCIRef, "kernel-image", "k", "Specify an OCI image containing the kernel at /boot/vmlinux and optionally, modules")
cmdutil.NetworkModeVar(fs, &cf.VM.Spec.Network.Mode)
cmdutil.SSHVar(fs, &cf.SSH)
cmdutil.VolumeVarP(fs, &cf.VM.Spec.Storage, "volumes", "v", "Expose block devices from the host inside the VM")
Expand Down
6 changes: 3 additions & 3 deletions cmd/ignite/run/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (cf *CreateFlags) constructVMFromCLI(args []string) error {
return err
}

cf.VM.Spec.Image.OCIClaim.Ref = ociRef
cf.VM.Spec.Image.OCIRef = ociRef
}

// Parse the --copy-files flag
Expand Down Expand Up @@ -91,7 +91,7 @@ func (cf *CreateFlags) NewCreateOptions(args []string) (*createOptions, error) {

// Get the image, or import it if it doesn't exist
var err error
co.image, err = operations.FindOrImportImage(providers.Client, cf.VM.Spec.Image.OCIClaim.Ref)
co.image, err = operations.FindOrImportImage(providers.Client, cf.VM.Spec.Image.OCIRef)
if err != nil {
return nil, err
}
Expand All @@ -100,7 +100,7 @@ func (cf *CreateFlags) NewCreateOptions(args []string) (*createOptions, error) {
cf.VM.SetImage(co.image)

// Get the kernel, or import it if it doesn't exist
co.kernel, err = operations.FindOrImportKernel(providers.Client, cf.VM.Spec.Kernel.OCIClaim.Ref)
co.kernel, err = operations.FindOrImportKernel(providers.Client, cf.VM.Spec.Kernel.OCIRef)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/ignite/run/ps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func Ps(po *psOptions) error {

o.Write("VM ID", "IMAGE", "KERNEL", "SIZE", "CPUS", "MEMORY", "CREATED", "STATUS", "IPS", "PORTS", "NAME")
for _, vm := range po.allVMs {
o.Write(vm.GetUID(), vm.Spec.Image.OCIClaim.Ref, vm.Spec.Kernel.OCIClaim.Ref,
o.Write(vm.GetUID(), vm.Spec.Image.OCIRef, vm.Spec.Kernel.OCIRef,
vm.Spec.DiskSize, vm.Spec.CPUs, vm.Spec.Memory, vm.GetCreated(), formatStatus(vm), vm.Status.IPAddresses,
vm.Spec.Network.Ports, vm.GetName())
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/ignite/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ func GetImageSourceTypes() []ImageSourceType {

// SetImage populates relevant fields to an Image on the VM object
func (vm *VM) SetImage(image *Image) {
vm.Spec.Image.OCIClaim = image.Spec.OCIClaim
vm.Spec.Image.OCIRef = image.Spec.OCIRef
vm.Status.Image = image.Status.OCISource
}

// SetKernel populates relevant fields to a Kernel on the VM object
func (vm *VM) SetKernel(kernel *Kernel) {
vm.Spec.Kernel.OCIClaim = kernel.Spec.OCIClaim
vm.Spec.Kernel.OCIRef = kernel.Spec.OCIRef
vm.Status.Kernel = kernel.Status.OCISource
}

Expand Down
21 changes: 5 additions & 16 deletions pkg/apis/ignite/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type Image struct {

// ImageSpec declares what the image contains
type ImageSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
OCIRef meta.OCIImageRef `json:"oci"`
}

// ImageSourceType is an enum of different supported Image Source Types
Expand All @@ -38,17 +38,6 @@ const (
ImageSourceTypeDocker ImageSourceType = "Docker"
)

// OCIImageClaim defines a claim for importing an OCI image
type OCIImageClaim struct {
// Type defines how the image should be imported
Type ImageSourceType `json:"type"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Also remove the ImageSourceType enum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 9d6ab9e.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also removed GetImageSourceTypes as it's redundant.

// Ref defines the reference to use when talking to the backend.
// This is most commonly the image name, followed by a tag.
// Other supported ways are $registry/$user/$image@sha256:$digest
// This ref is also used as ObjectMeta.Name for kinds Images and Kernels
Ref meta.OCIImageRef `json:"ref"`
}

// OCIImageSource specifies how the OCI image was imported.
// It is the status variant of OCIImageClaim
type OCIImageSource struct {
Expand Down Expand Up @@ -136,7 +125,7 @@ type Kernel struct {

// KernelSpec describes the properties of a kernel
type KernelSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
OCIRef meta.OCIImageRef `json:"oci"`
// Optional future feature, support per-kernel specific default command lines
// DefaultCmdLine string
}
Expand Down Expand Up @@ -183,12 +172,12 @@ type VMSpec struct {
}

type VMImageSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
OCIRef meta.OCIImageRef `json:"oci"`
}

type VMKernelSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
CmdLine string `json:"cmdLine,omitempty"`
OCIRef meta.OCIImageRef `json:"oci"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally, you always want to match the field name with the JSON tag to avoid so-called "OpenAPI violations".
Let's change this name to avoid those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 9d6ab9e.

CmdLine string `json:"cmdLine,omitempty"`
}

type VMNetworkSpec struct {
Expand Down
8 changes: 2 additions & 6 deletions pkg/apis/ignite/v1alpha2/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ func addDefaultingFuncs(scheme *runtime.Scheme) error {
return RegisterDefaults(scheme)
}

func SetDefaults_OCIImageClaim(obj *OCIImageClaim) {
obj.Type = ImageSourceTypeDocker
}

func SetDefaults_PoolSpec(obj *PoolSpec) {
if obj.AllocationSize == meta.EmptySize {
obj.AllocationSize = meta.NewSizeFromSectors(constants.POOL_ALLOCATION_SIZE_SECTORS)
Expand Down Expand Up @@ -52,8 +48,8 @@ func SetDefaults_VMSpec(obj *VMSpec) {

func SetDefaults_VMKernelSpec(obj *VMKernelSpec) {
// Default the kernel image if unset
if len(obj.OCIClaim.Ref) == 0 {
obj.OCIClaim.Ref, _ = meta.NewOCIImageRef(constants.DEFAULT_KERNEL_IMAGE)
if len(obj.OCIRef) == 0 {
obj.OCIRef, _ = meta.NewOCIImageRef(constants.DEFAULT_KERNEL_IMAGE)
}

if len(obj.CmdLine) == 0 {
Expand Down
21 changes: 5 additions & 16 deletions pkg/apis/ignite/v1alpha2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type Image struct {

// ImageSpec declares what the image contains
type ImageSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
OCIRef meta.OCIImageRef `json:"oci"`
}

// ImageSourceType is an enum of different supported Image Source Types
Expand All @@ -38,17 +38,6 @@ const (
ImageSourceTypeDocker ImageSourceType = "Docker"
)

// OCIImageClaim defines a claim for importing an OCI image
type OCIImageClaim struct {
// Type defines how the image should be imported
Type ImageSourceType `json:"type"`
// Ref defines the reference to use when talking to the backend.
// This is most commonly the image name, followed by a tag.
// Other supported ways are $registry/$user/$image@sha256:$digest
// This ref is also used as ObjectMeta.Name for kinds Images and Kernels
Ref meta.OCIImageRef `json:"ref"`
}

// OCIImageSource specifies how the OCI image was imported.
// It is the status variant of OCIImageClaim
type OCIImageSource struct {
Expand Down Expand Up @@ -136,7 +125,7 @@ type Kernel struct {

// KernelSpec describes the properties of a kernel
type KernelSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
OCIRef meta.OCIImageRef `json:"oci"`
// Optional future feature, support per-kernel specific default command lines
// DefaultCmdLine string
}
Expand Down Expand Up @@ -183,12 +172,12 @@ type VMSpec struct {
}

type VMImageSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
OCIRef meta.OCIImageRef `json:"oci"`
}

type VMKernelSpec struct {
OCIClaim OCIImageClaim `json:"ociClaim"`
CmdLine string `json:"cmdLine,omitempty"`
OCIRef meta.OCIImageRef `json:"oci"`
CmdLine string `json:"cmdLine,omitempty"`
}

type VMNetworkSpec struct {
Expand Down
23 changes: 11 additions & 12 deletions pkg/apis/ignite/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,21 @@ import (
// ValidateVM validates a VM object and collects all encountered errors
func ValidateVM(obj *api.VM) (allErrs field.ErrorList) {
allErrs = append(allErrs, ValidateNetworkMode(obj.Spec.Network.Mode, field.NewPath(".spec.network.mode"))...)
allErrs = append(allErrs, ValidateOCIImageClaim(&obj.Spec.Image.OCIClaim, field.NewPath(".spec.image.ociClaim"))...)
allErrs = append(allErrs, ValidateOCIImageClaim(&obj.Spec.Kernel.OCIClaim, field.NewPath(".spec.kernel.ociClaim"))...)
allErrs = append(allErrs, RequireOCIImageRef(&obj.Spec.Image.OCIRef, field.NewPath(".spec.image.oci"))...)
allErrs = append(allErrs, RequireOCIImageRef(&obj.Spec.Kernel.OCIRef, field.NewPath(".spec.kernel.oci"))...)
allErrs = append(allErrs, ValidateFileMappings(&obj.Spec.CopyFiles, field.NewPath(".spec.copyFiles"))...)
allErrs = append(allErrs, ValidateVMStorage(&obj.Spec.Storage, field.NewPath(".spec.storage"))...)
// TODO: Add vCPU, memory, disk max and min sizes
// TODO: Add port mapping validation
return
}

// ValidateOCIImageClaim validates an OCI image claim
func ValidateOCIImageClaim(c *api.OCIImageClaim, fldPath *field.Path) (allErrs field.ErrorList) {
allErrs = append(allErrs, ValidateImageSourceType(c.Type, fldPath.Child("type"))...)
allErrs = append(allErrs, RequireOCIImageRef(&c.Ref, fldPath.Child("ref"))...)

return
}

// RequireOCIImageRef
// RequireOCIImageRef validates that the OCIImageRef is set
func RequireOCIImageRef(ref *meta.OCIImageRef, fldPath *field.Path) (allErrs field.ErrorList) {
if ref.IsUnset() {
allErrs = append(allErrs, field.Required(fldPath, "ociRef is mandatory"))
allErrs = append(allErrs, field.Required(fldPath, "the OCI reference is mandatory"))
}

return
}

Expand All @@ -45,6 +38,7 @@ func ValidateFileMappings(mappings *[]api.FileMapping, fldPath *field.Path) (all
allErrs = append(allErrs, ValidateAbsolutePath(mapping.HostPath, mappingPath.Child("hostPath"))...)
allErrs = append(allErrs, ValidateAbsolutePath(mapping.VMPath, mappingPath.Child("vmPath"))...)
}

return
}

Expand All @@ -62,6 +56,7 @@ func ValidateAbsolutePath(pathStr string, fldPath *field.Path) (allErrs field.Er
if !path.IsAbs(pathStr) {
allErrs = append(allErrs, field.Invalid(fldPath, pathStr, "path must be absolute"))
}

return
}

Expand All @@ -74,9 +69,11 @@ func ValidateNetworkMode(mode api.NetworkMode, fldPath *field.Path) (allErrs fie
found = true
}
}

if !found {
allErrs = append(allErrs, field.Invalid(fldPath, mode, fmt.Sprintf("network mode must be one of %v", modes)))
}

return
}

Expand All @@ -89,8 +86,10 @@ func ValidateImageSourceType(t api.ImageSourceType, fldPath *field.Path) (allErr
found = true
}
}

if !found {
allErrs = append(allErrs, field.Invalid(fldPath, t, fmt.Sprintf("image source type must be one of %v", types)))
}

return
}
16 changes: 9 additions & 7 deletions pkg/apis/meta/v1alpha1/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ func NewOCIImageRef(imageStr string) (OCIImageRef, error) {
if err != nil {
return "", err
}

namedTagged, ok := named.(reference.NamedTagged)
if !ok {
return "", fmt.Errorf("could not parse image name with a tag %s", imageStr)
return "", fmt.Errorf("could not parse image %q with a tag", imageStr)
}

return OCIImageRef(reference.FamiliarString(namedTagged)), nil
}

// OCIImageRef is a string by which an OCI runtime can identify an image to retrieve.
// It needs to have a tag and usually looks like "weaveworks/ignite-ubuntu:latest".
type OCIImageRef string

var _ fmt.Stringer = OCIImageRef("")
Expand All @@ -40,15 +44,13 @@ func (i OCIImageRef) IsUnset() bool {
return len(i) == 0
}

func (i *OCIImageRef) UnmarshalJSON(b []byte) error {
var str string
var err error

if err = json.Unmarshal(b, &str); err != nil {
func (i *OCIImageRef) UnmarshalJSON(b []byte) (err error) {
var s string
if err = json.Unmarshal(b, &s); err != nil {
return err
}

*i, err = NewOCIImageRef(str)
*i, err = NewOCIImageRef(s)
return err
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/operations/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func importImage(c *client.Client, ociRef meta.OCIImageRef) (*api.Image, error)
// Set the image name
image.Name = ociRef.String()
// Set the image's ociRef
image.Spec.OCIClaim.Ref = ociRef
image.Spec.OCIRef = ociRef
// Set the image's ociSource
image.Status.OCISource = *src

Expand Down Expand Up @@ -112,7 +112,7 @@ func importKernel(c *client.Client, ociRef meta.OCIImageRef) (*api.Kernel, error
// Set the kernel name
kernel.Name = ociRef.String()
// Set the kernel's ociRef
kernel.Spec.OCIClaim.Ref = ociRef
kernel.Spec.OCIRef = ociRef
// Set the kernel's ociSource
kernel.Status.OCISource = *src

Expand Down
10 changes: 6 additions & 4 deletions pkg/operations/lookup/lookup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,19 @@ import (
)

func ImageUIDForVM(vm *api.VM, c *client.Client) (meta.UID, error) {
image, err := c.Images().Find(filter.NewNameFilter(vm.Spec.Image.OCIClaim.Ref.String()))
image, err := c.Images().Find(filter.NewNameFilter(vm.Spec.Image.OCIRef.String()))
if err != nil {
return meta.UID(""), err
return "", err
}

return image.GetUID(), nil
}

func KernelUIDForVM(vm *api.VM, c *client.Client) (meta.UID, error) {
kernel, err := c.Kernels().Find(filter.NewNameFilter(vm.Spec.Kernel.OCIClaim.Ref.String()))
kernel, err := c.Kernels().Find(filter.NewNameFilter(vm.Spec.Kernel.OCIRef.String()))
if err != nil {
return meta.UID(""), err
return "", err
}

return kernel.GetUID(), nil
}