Skip to content

Commit

Permalink
very satisfying deletion of things.
Browse files Browse the repository at this point in the history
Signed-off-by: vsoch <[email protected]>
  • Loading branch information
vsoch committed Sep 20, 2023
1 parent f525b8a commit 8084830
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 156 deletions.
126 changes: 0 additions & 126 deletions api/v1alpha1/metric_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,50 +129,6 @@ type MetricAddon struct {
MapOptions map[string]map[string]intstr.IntOrString `json:"mapOptions"`
}

// Storage that will be monitored, or storage alongside a standalone metric
type Storage struct {

// Volume type to test (not all storage interfaces require one explicitly)
//+optional
Volume Volume `json:"volume"`

// Commands to run (pre is supported to make bind)
// +optional
Commands Commands `json:"commands"`
}

// Application that will be monitored
type Application struct {
Image string `json:"image"`

// command to execute and monitor (if consistent across pods)
Command string `json:"command"`

// Working Directory
//+optional
WorkingDir string `json:"workingDir"`

// Entrypoint of container, if different from command
//+optional
Entrypoint string `json:"entrypoint"`

// A pull secret for the application container
//+optional
PullSecret string `json:"pullSecret"`

// Resources include limits and requests for the application
// +optional
Resources ContainerResources `json:"resources"`

// Container Spec has attributes for the container
//+optional
Attributes ContainerSpec `json:"attributes"`

// Existing Volumes for the application
// +optional
Volumes map[string]Volume `json:"volumes"`
}

// ContainerResources include limits and requests
type ContainerResources struct {

Expand Down Expand Up @@ -200,47 +156,6 @@ type Commands struct {

type ContainerResource map[string]intstr.IntOrString

// A Volume should correspond with an existing volume, either:
// config map, secret, or claim name.
type Volume struct {

// Path and claim name are always required if a secret isn't defined
// +optional
Path string `json:"path,omitempty"`

// Hostpath volume on the host to bind to path
// +optional
HostPath string `json:"hostPath"`

// Config map name if the existing volume is a config map
// You should also define items if you are using this
// +optional
ConfigMapName string `json:"configMapName,omitempty"`

// Items (key and paths) for the config map
// +optional
Items map[string]string `json:"items"`

// Claim name if the existing volume is a PVC
// +optional
ClaimName string `json:"claimName,omitempty"`

// An existing secret
// +optional
SecretName string `json:"secretName,omitempty"`

// EmptyVol if true generates an empty volume at the path
// +kubebuilder:default=false
// +default=false
// +optional
EmptyVol bool `json:"emptyVol,omitempty"`

// +kubebuilder:default=false
// +default=false
// +optional
ReadOnly bool `json:"readOnly,omitempty"`
}

// The difference between benchmark and metric is subtle.
// A metric is more a measurment, and the benchmark is the comparison value.
// I don't have strong opinions but I think we are doing more measurment
Expand Down Expand Up @@ -325,47 +240,6 @@ func (m *MetricSet) Validate() bool {
return true
}

// validateExistingVolumes ensures secret names vs. volume paths are valid
func (m *MetricSet) validateVolumes(volumes map[string]Volume) bool {

valid := true
for key, volume := range volumes {

// Case 1: it's a secret and we only need that
if volume.SecretName != "" {
continue
}

// Case 2: it's a config map (and will have items too, but we don't hard require them)
if volume.ConfigMapName != "" {
continue
}

// Case 3: Hostpath volume (mostly for testing)
if volume.HostPath != "" {
continue
}

// Case 4: claim desired without path
if volume.ClaimName == "" && volume.Path != "" {
fmt.Printf("😥️ Found existing volume %s with path %s that is missing a claim name\n", key, volume.Path)
valid = false
}
// Case 5: reverse of the above
if volume.ClaimName != "" && volume.Path == "" {
fmt.Printf("😥️ Found existing volume %s with claimName %s that is missing a path\n", key, volume.ClaimName)
valid = false
}

// Case 6: empty volume needs path
if volume.EmptyVol && volume.Path == "" {
fmt.Printf("😥️ Found empty volume %s that is missing a path\n", key)
valid = false
}
}
return valid
}

//+kubebuilder:object:root=true

// MetricSetList contains a list of MetricSet
Expand Down
15 changes: 6 additions & 9 deletions controllers/metric/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

api "github.com/converged-computing/metrics-operator/api/v1alpha1"
mctrl "github.com/converged-computing/metrics-operator/pkg/metrics"
"github.com/converged-computing/metrics-operator/pkg/specs"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand All @@ -26,6 +27,7 @@ func (r *MetricSetReconciler) ensureConfigMaps(
ctx context.Context,
spec *api.MetricSet,
set *mctrl.MetricSet,
containerSpecs []*specs.ContainerSpec,
) (*corev1.ConfigMap, ctrl.Result, error) {

// Look for the config map by name
Expand All @@ -47,15 +49,10 @@ func (r *MetricSetReconciler) ensureConfigMaps(
// or possible multiple for a standalone metric
data := map[string]string{}

// Go through each entrypoint script
// TODO update this using the containerspec provided above
/*for count, es := range set.EntrypointScripts(spec) {
key := es.Name
if key == "" {
key = fmt.Sprintf("entrypoint-%d", count)
}
data[key] = es.Script
}*/
// Go through each container spec entrypoint
for _, cs := range containerSpecs {
data[cs.EntrypointScript.Name] = cs.EntrypointScript.Script
}

cm, result, err := r.getConfigMap(ctx, spec, data)
if err != nil {
Expand Down
24 changes: 11 additions & 13 deletions controllers/metric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ func (r *MetricSetReconciler) ensureMetricSet(
}

// Ensure we create the JobSet for the MetricSet
// either application, storage, or standalone based
// This could be updated to support > 1
scripts, _, result, err = r.ensureJobSet(ctx, spec, set)
// We get back container specs to use for generating configmaps
_, cs, result, err := r.ensureJobSet(ctx, spec, set)
if err != nil {
return result, err
}
Expand All @@ -47,7 +46,7 @@ func (r *MetricSetReconciler) ensureMetricSet(
// and named by metric index or custom metric script key name
// We could theoretically allow creating more than one JobSet here
// and change the name to include the group type.
_, result, err := r.ensureConfigMaps(ctx, spec, set, cms)
_, result, err = r.ensureConfigMaps(ctx, spec, set, cs)
if err != nil {
return result, err
}
Expand Down Expand Up @@ -78,14 +77,14 @@ func (r *MetricSetReconciler) ensureJobSet(
ctx context.Context,
spec *api.MetricSet,
set *mctrl.MetricSet,
) ([]*jobset.JobSet, []specs.EntrypointScript, ctrl.Result, error) {
) ([]*jobset.JobSet, []*specs.ContainerSpec, ctrl.Result, error) {

// Look for an existing job
// We only care about the set Name/Namespace matched to one
// This can eventually update to support > 1 if needed
existing, err := r.getExistingJob(ctx, spec)
jobsets := []*jobset.JobSet{existing}
scripts := []specs.EntrypointScript{}
cs := []*specs.ContainerSpec{}

// Create a new job if it does not exist
if err != nil {
Expand All @@ -97,19 +96,18 @@ func (r *MetricSetReconciler) ensureJobSet(
)

// Get one JobSet to create (can eventually support > 1)
// The scripts are paired here. If we already made the jobsets,
// the scripts (config maps) should exist too.
jobsets, scripts, err := mctrl.GetJobSet(spec, set)
// container specs allow us to create config maps
jobsets, cs, err := mctrl.GetJobSet(spec, set)
if err != nil {
return jobsets, scripts, ctrl.Result{}, err
return jobsets, cs, ctrl.Result{}, err
}
for _, js := range jobsets {
err = r.createJobSet(ctx, spec, js)
if err != nil {
return jobsets, scripts, ctrl.Result{}, err
return jobsets, cs, ctrl.Result{}, err
}
}
return jobsets, scripts, ctrl.Result{}, err
return jobsets, cs, ctrl.Result{}, err

} else {
r.Log.Info(
Expand All @@ -118,7 +116,7 @@ func (r *MetricSetReconciler) ensureJobSet(
"Name:", existing.Name,
)
}
return jobsets, scripts, ctrl.Result{}, err
return jobsets, cs, ctrl.Result{}, err
}

// createJobSet handles the creation operator
Expand Down
5 changes: 0 additions & 5 deletions pkg/metrics/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@ func (m LauncherWorker) Name() string {
return m.Identifier
}

// GetVolumes (if necessary) this is likely only for application metric types
func (m LauncherWorker) GetVolumes() map[string]api.Volume {
return map[string]api.Volume{}
}

// Description returns the metric description
func (m LauncherWorker) Description() string {
return m.Summary
Expand Down
3 changes: 0 additions & 3 deletions pkg/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ type Metric interface {
Image() string
WorkingDir() string

// Customizations / exposure for other containers in the JobSet
GetVolumes() map[string]api.Volume

// Options and exportable attributes
SetOptions(*api.Metric)
Options() map[string]intstr.IntOrString
Expand Down

0 comments on commit 8084830

Please sign in to comment.