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 support for Profiles #1903

Merged
merged 7 commits into from
Dec 20, 2023
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
1 change: 1 addition & 0 deletions docs/cmd/kn_service_apply.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ kn service apply s0 --filename my-svc.yml
--probe-liveness-opts string Add common options to liveness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
--probe-readiness string Add readiness probe to Service deployment. Supported probe types are HTTGet, Exec and TCPSocket. Format: [http,https]:host:port:path, exec:cmd[,cmd,...], tcp:host:port.
--probe-readiness-opts string Add common options to readiness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
--profile string The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service.To unset, specify the profile name followed by a "-" (e.g., name-).
--pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent
--pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
--request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'.
Expand Down
4 changes: 4 additions & 0 deletions docs/cmd/kn_service_create.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ kn service create NAME --image IMAGE
kn service create gitopstest -n test-ns --image knativesamples/helloworld --target=/user/knfiles
kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.yaml
kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.json

# Create a service with profile
kn service create profiletest --image knativesamples/helloworld --profile istio
```

### Options
Expand Down Expand Up @@ -85,6 +88,7 @@ kn service create NAME --image IMAGE
--probe-liveness-opts string Add common options to liveness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
--probe-readiness string Add readiness probe to Service deployment. Supported probe types are HTTGet, Exec and TCPSocket. Format: [http,https]:host:port:path, exec:cmd[,cmd,...], tcp:host:port.
--probe-readiness-opts string Add common options to readiness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
--profile string The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service.To unset, specify the profile name followed by a "-" (e.g., name-).
--pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent
--pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
--request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'.
Expand Down
1 change: 1 addition & 0 deletions docs/cmd/kn_service_update.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ kn service update NAME
--probe-liveness-opts string Add common options to liveness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
--probe-readiness string Add readiness probe to Service deployment. Supported probe types are HTTGet, Exec and TCPSocket. Format: [http,https]:host:port:path, exec:cmd[,cmd,...], tcp:host:port.
--probe-readiness-opts string Add common options to readiness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
--profile string The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service.To unset, specify the profile name followed by a "-" (e.g., name-).
--pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent
--pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
--request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'.
Expand Down
64 changes: 64 additions & 0 deletions pkg/kn/commands/service/configuration_edit_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/serving/pkg/apis/config"

knconfig "knative.dev/client/pkg/kn/config"
knflags "knative.dev/client/pkg/kn/flags"
servinglib "knative.dev/client/pkg/serving"
"knative.dev/client/pkg/util"
Expand Down Expand Up @@ -57,6 +58,7 @@ type ConfigurationEditFlags struct {
ClusterLocal bool
ScaleInit int
TimeoutSeconds int64
Profile string

// Preferences about how to do the action.
LockToDigest bool
Expand Down Expand Up @@ -187,6 +189,11 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
"Duration in seconds that the request routing layer will wait for a request delivered to a "+""+
"container to begin replying")
p.markFlagMakesRevision("timeout")

command.Flags().StringVar(&p.Profile, "profile", "",
"The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service."+
"To unset, specify the profile name followed by a \"-\" (e.g., name-).")
sharmaansh21 marked this conversation as resolved.
Show resolved Hide resolved
p.markFlagMakesRevision("profile")
}

// AddUpdateFlags adds the flags specific to update.
Expand Down Expand Up @@ -479,6 +486,63 @@ func (p *ConfigurationEditFlags) Apply(
service.Spec.Template.Spec.TimeoutSeconds = &p.TimeoutSeconds
}

if cmd.Flags().Changed("profile") {
profileName := ""
deleteProfile := false
if strings.HasSuffix(p.Profile, "-") {
profileName = p.Profile[:len(p.Profile)-1]
deleteProfile = true
sharmaansh21 marked this conversation as resolved.
Show resolved Hide resolved
} else {
profileName = p.Profile
deleteProfile = false
}

if len(knconfig.GlobalConfig.Profile(profileName).Annotations) > 0 || len(knconfig.GlobalConfig.Profile(profileName).Labels) > 0 {
annotations := knconfig.GlobalConfig.Profile(profileName).Annotations
labels := knconfig.GlobalConfig.Profile(profileName).Labels

profileAnnotations := make(util.StringMap)
for _, value := range annotations {
profileAnnotations[value.Name] = value.Value
}

profileLabels := make(util.StringMap)
for _, value := range labels {
profileLabels[value.Name] = value.Value
}

if deleteProfile {
var annotationsToRemove []string
for _, value := range annotations {
annotationsToRemove = append(annotationsToRemove, value.Name)
}

var labelsToRemove []string
for _, value := range labels {
labelsToRemove = append(labelsToRemove, value.Name)
}

if err = servinglib.UpdateRevisionTemplateAnnotations(template, map[string]string{}, annotationsToRemove); err != nil {
return err
}
updatedLabels := servinglib.UpdateLabels(service.ObjectMeta.Labels, map[string]string{}, labelsToRemove)
service.ObjectMeta.Labels = updatedLabels // In case service.ObjectMeta.Labels was nil
} else {
if err = servinglib.UpdateRevisionTemplateAnnotations(template, profileAnnotations, []string{}); err != nil {
return err
}
updatedLabels := servinglib.UpdateLabels(service.ObjectMeta.Labels, profileLabels, []string{})
service.ObjectMeta.Labels = updatedLabels // In case service.ObjectMeta.Labels was nil
}

if err != nil {
return err
}
} else {
return fmt.Errorf("profile %s doesn't exist", profileName)
}
}

return nil
}

Expand Down
140 changes: 140 additions & 0 deletions pkg/kn/commands/service/configuration_edit_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@
package service

import (
"os"
"path/filepath"
"testing"

"github.com/spf13/viper"
"gotest.tools/v3/assert"

"knative.dev/client/pkg/kn/commands"
"knative.dev/client/pkg/kn/config"
"knative.dev/client/pkg/util"
"knative.dev/serving/pkg/apis/autoscaling"
)
Expand Down Expand Up @@ -75,3 +80,138 @@ func TestScaleActivation(t *testing.T) {
assert.NilError(t, err)
assert.Equal(t, svc.Spec.Template.Annotations[autoscaling.ActivationScaleKey], "2")
}

func TestApplyDefaultProfileFlag(t *testing.T) {
var editFlags ConfigurationEditFlags
knParams := &commands.KnParams{}
cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)

editFlags.AddCreateFlags(cmd)

err := config.BootstrapConfig()
assert.NilError(t, err)

svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
cmd.SetArgs([]string{"--profile", "istio"})
cmd.Execute()
editFlags.Apply(&svc, nil, cmd)
assert.Equal(t, svc.Spec.Template.Annotations["sidecar.istio.io/inject"], "true")
assert.Equal(t, svc.Spec.Template.Annotations["sidecar.istio.io/rewriteAppHTTPProbers"], "true")
assert.Equal(t, svc.Spec.Template.Annotations["serving.knative.openshift.io/enablePassthrough"], "true")
}

func TestApplyProfileFlag(t *testing.T) {
var editFlags ConfigurationEditFlags
knParams := &commands.KnParams{}
cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
configYaml := `
profiles:
testprofile:
labels:
- name: environment
value: "test"
annotations:
- name: sidecar.testprofile.io/inject
value: "true"
- name: sidecar.testprofile.io/rewriteAppHTTPProbers
value: "true"
`
_, cleanup := setupConfig(t, configYaml)
defer cleanup()

editFlags.AddCreateFlags(cmd)

err := config.BootstrapConfig()
assert.NilError(t, err)

svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
cmd.SetArgs([]string{"--profile", "testprofile"})
cmd.Execute()
editFlags.Apply(&svc, nil, cmd)
assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/inject"], "true")
assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/rewriteAppHTTPProbers"], "true")
assert.Equal(t, svc.ObjectMeta.Labels["environment"], "test")
}

func TestDeleteProfileFlag(t *testing.T) {
var editFlags ConfigurationEditFlags
knParams := &commands.KnParams{}
cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
configYaml := `
profiles:
testprofile:
labels:
- name: environment
value: "test"
annotations:
- name: sidecar.testprofile.io/inject
value: "true"
- name: sidecar.testprofile.io/rewriteAppHTTPProbers
value: "true"
`
_, cleanup := setupConfig(t, configYaml)
defer cleanup()

editFlags.AddCreateFlags(cmd)

err := config.BootstrapConfig()
assert.NilError(t, err)

svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
cmd.SetArgs([]string{"--profile", "testprofile"})
cmd.Execute()
editFlags.Apply(&svc, nil, cmd)
assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/inject"], "true")
assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/rewriteAppHTTPProbers"], "true")
assert.Equal(t, svc.ObjectMeta.Labels["environment"], "test")

cmd.SetArgs([]string{"--profile", "testprofile-"})
cmd.Execute()
editFlags.Apply(&svc, nil, cmd)

assert.Equal(t, svc.Spec.Template.Annotations["sidecar.istio.io/inject"], "")
assert.Equal(t, len(svc.Spec.Template.Annotations), 1)

assert.Equal(t, svc.ObjectMeta.Labels["environment"], "")
}

func TestApplyProfileFlagError(t *testing.T) {
var editFlags ConfigurationEditFlags
knParams := &commands.KnParams{}
cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
editFlags.AddCreateFlags(cmd)
err := config.BootstrapConfig()
assert.NilError(t, err)

svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
cmd.SetArgs([]string{"--profile", "invalidprofile"})
cmd.Execute()
err = editFlags.Apply(&svc, nil, cmd)
assert.Assert(t, util.ContainsAll(err.Error(), "profile", "invalidprofile"))
}

func setupConfig(t *testing.T, configContent string) (string, func()) {
sharmaansh21 marked this conversation as resolved.
Show resolved Hide resolved
tmpDir := t.TempDir()

// Avoid to be fooled by the things in the the real homedir
oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)

// Save old args
backupArgs := os.Args

// WriteCache out a temporary configContent file
var cfgFile string
if configContent != "" {
cfgFile = filepath.Join(tmpDir, "config.yaml")
os.Args = []string{"kn", "--config", cfgFile}
err := os.WriteFile(cfgFile, []byte(configContent), 0644)
assert.NilError(t, err)
}
return cfgFile, func() {
// Cleanup everything
os.Setenv("HOME", oldHome)
os.Args = backupArgs
viper.Reset()
}
sharmaansh21 marked this conversation as resolved.
Show resolved Hide resolved
}
5 changes: 4 additions & 1 deletion pkg/kn/commands/service/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ var create_example = `
# Create the service in offline mode instead of kubernetes cluster (Beta)
kn service create gitopstest -n test-ns --image knativesamples/helloworld --target=/user/knfiles
kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.yaml
kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.json`
kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.json

# Create a service with profile
kn service create profiletest --image knativesamples/helloworld --profile istio`

func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
var editFlags ConfigurationEditFlags
Expand Down
56 changes: 56 additions & 0 deletions pkg/kn/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ type config struct {

// channelTypeMappings is a list of channel type mapping
channelTypeMappings []ChannelTypeMapping

// profiles is a map of profiles from the config file and built-in profiles
profiles map[string]Profile
}

func (c *config) ContextSharing() bool {
Expand Down Expand Up @@ -100,6 +103,10 @@ func (c *config) SinkMappings() []SinkMapping {
return c.sinkMappings
}

func (c *config) Profile(profile string) Profile {
return c.profiles[profile]
}

func (c *config) ChannelTypeMappings() []ChannelTypeMapping {
return c.channelTypeMappings
}
Expand Down Expand Up @@ -167,6 +174,12 @@ func BootstrapConfig() error {
return err
}

// Deserialize profiles if configured
err = parseProfiles()
if err != nil {
return err
}

// Deserialize channel type mappings if configured
err = parseChannelTypeMappings()
return err
Expand Down Expand Up @@ -267,6 +280,49 @@ func parseSinkMappings() error {
return nil
}

// parse profiles and store them in the global configuration
func parseProfiles() error {
if viper.IsSet(profiles) {
err := viper.UnmarshalKey(profiles, &globalConfig.profiles)
if err != nil {
return fmt.Errorf("error while parsing profiles in configuration file %s: %w",
viper.ConfigFileUsed(), err)
}
}
globalConfig.profiles = mergeProfilesWithBuiltInProfiles(globalConfig.profiles)

return nil
}

// defaultProfiles returns the built-in profiles
func builtInProfiles() map[string]Profile {
return map[string]Profile{
istio: {
Annotations: []NamedValue{
{Name: "sidecar.istio.io/inject", Value: "true"},
{Name: "sidecar.istio.io/rewriteAppHTTPProbers", Value: "true"},
{Name: "serving.knative.openshift.io/enablePassthrough", Value: "true"},
},
},
}
}

// mergeProfilesWithDefaultProfiles merges the given profiles with the built-in profiles
func mergeProfilesWithBuiltInProfiles(profiles map[string]Profile) map[string]Profile {
builtInProfiles := builtInProfiles()
mergedProfiles := make(map[string]Profile, len(builtInProfiles)+len(profiles))

for key, value := range builtInProfiles {
mergedProfiles[key] = value
}

for key, value := range profiles {
mergedProfiles[key] = value
}

return mergedProfiles
}

// parse channel type mappings and store them in the global configuration
func parseChannelTypeMappings() error {
if viper.IsSet(keyChannelTypeMappings) {
Expand Down
Loading
Loading