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.

Sign up for GitHub

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

Issue 495: Validate Pravega manifest settings before deployment #584

Closed
wants to merge 1 commit into from
Closed

Issue 495: Validate Pravega manifest settings before deployment #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 67 additions & 1 deletion pkg/apis/pravega/v1beta1/pravegacluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,15 @@ func (p *PravegaCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (p *PravegaCluster) ValidateCreate() error {
log.Printf("validate create %s", p.Name)
return p.ValidatePravegaVersion()
err := p.ValidatePravegaVersion()
if err != nil {
return err
}
err = p.ValidateSegmentStore()
if err != nil {
return err
}
return nil
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
Expand All @@ -911,6 +919,10 @@ func (p *PravegaCluster) ValidateUpdate(old runtime.Object) error {
if err != nil {
return err
}
err = p.ValidateSegmentStore()
if err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -1194,6 +1206,60 @@ func (p *PravegaCluster) validateConfigMap() error {
return nil
}

func (p *PravegaCluster) ValidateSegmentStore() error {
totalMemoryQuantity := p.Spec.Pravega.SegmentStoreResources.Requests[corev1.ResourceMemory]
if (resource.Quantity{}) == totalMemoryQuantity {
return fmt.Errorf("Missing required value for field .spec.pravega.segmentStoreResources.requests.memory")
}

cacheSizeString := p.Spec.Pravega.Options["pravegaservice.cache.size.max"]
if cacheSizeString == "" {
return fmt.Errorf("Missing required value for option pravegaservice.cache.size.max")
}
Comment on lines +1210 to +1218
Copy link

Choose a reason for hiding this comment

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

I'm not sure we need to make these two values required.
If they are to be required, they need to be made required in the CRD and the requirement needs to be documented.

cacheSizeQuantity := resource.MustParse(cacheSizeString)
maxDirectMemoryString := ""
xmxString := ""

for _, value := range p.Spec.Pravega.SegmentStoreJVMOptions {
if strings.Contains(value, "-Xmx") {
xmxString = strings.ToUpper(strings.TrimPrefix(value, "-Xmx")) + "i"
}

if strings.Contains(value, "-XX:MaxDirectMemorySize=") {
maxDirectMemoryString = strings.ToUpper(strings.TrimPrefix(value, "-XX:MaxDirectMemorySize=")) + "i"
}
}

if xmxString == "" {
return fmt.Errorf("Missing required value for Segment Store JVM Option -Xmx")
}
xmxQuantity := resource.MustParse(xmxString)

if maxDirectMemoryString == "" {
return fmt.Errorf("Missing required value for Segment Store JVM option -XX:MaxDirectMemorySize")
}
maxDirectMemoryQuantity := resource.MustParse(maxDirectMemoryString)

totalMemory := totalMemoryQuantity.Value()
xmx := xmxQuantity.Value()
maxDirectMemorySize := maxDirectMemoryQuantity.Value()
cacheSize := cacheSizeQuantity.Value()

if totalMemory <= maxDirectMemorySize+xmx {
return fmt.Errorf("MaxDirectMemorySize(%v B) along with JVM Xmx value(%v B) is greater than or equal to the total available memory(%v B)!", maxDirectMemorySize, xmx, totalMemory)
}

if maxDirectMemorySize <= xmx {
return fmt.Errorf("JVM Xmx(%v B) configured is greater than or equal to the JVM MaxDirectMemorySize(%v B) value", xmx, maxDirectMemorySize)
}

if maxDirectMemorySize <= cacheSize {
return fmt.Errorf("Cache size(%v B) configured is greater than or equal to the JVM MaxDirectMemorySize(%v B) value", cacheSize, maxDirectMemorySize)
}

return nil
}

//to return name of segmentstore based on the version
func (p *PravegaCluster) StatefulSetNameForSegmentstore() string {
if util.IsVersionBelow(p.Spec.Version, "0.7.0") {
Expand Down
55 changes: 27 additions & 28 deletions pkg/test/e2e/e2eutil/pravegacluster_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,35 +159,34 @@ func CreatePravegaClusterWithTlsAuth(t *testing.T, f *framework.Framework, ctx *
p.Spec.TLS.Static.ControllerSecret = "controller-tls"
p.Spec.TLS.Static.SegmentStoreSecret = "segmentstore-tls"
p.Spec.Pravega.Options = map[string]string{
"pravegaservice.containerCount": "4",
"pravegaservice.cacheMaxSize": "1073741824",
"pravegaservice.zkSessionTimeoutMs": "10000",
"attributeIndex.readBlockSize": "1048576",
"readIndex.storageReadAlignment": "1048576",
"durableLog.checkpointMinCommitCount": "300",
"bookkeeper.bkAckQuorumSize": "3",
"controller.auth.tlsEnabled": "true",
"controller.auth.tlsCertFile": "/etc/secret-volume/controller01.pem",
"controller.auth.tlsKeyFile": "/etc/secret-volume/controller01.key.pem",
"controller.auth.tlsTrustStore": "/etc/secret-volume/ca-cert",
"controller.rest.tlsKeyStoreFile": "/etc/secret-volume/controller01.jks",
"controller.rest.tlsKeyStorePasswordFile": "/etc/secret-volume/pass-secret-tls",
"controller.auth.userPasswordFile": "/etc/auth-passwd-volume/pass-secret-tls-auth.txt",
"pravegaservice.enableTls": "true",
"pravegaservice.certFile": "/etc/secret-volume/segmentstore01.pem",
"pravegaservice.keyFile": "/etc/secret-volume/segmentstore01.key.pem",
"autoScale.tlsEnabled": "true",
"autoScale.tlsCertFile": "/etc/secret-volume/ca-cert",
"bookkeeper.tlsEnabled": "true",
"bookkeeper.tlsTrustStorePath": "empty",
"autoScale.validateHostName": "true",
"autoScale.authEnabled": "true",
"controller.auth.tokenSigningKey": "secret",
"autoScale.tokenSigningKey": "secret",
"pravega.client.auth.token": "YWRtaW46MTExMV9hYWFh",
"pravega.client.auth.method": "Basic",
"pravegaservice.container.count": "4",
"pravegaservice.cache.size.max": "1610612736",
"pravegaservice.zk.connect.sessionTimeout.milliseconds": "10000",
"attributeIndex.readBlockSize": "1048576",
"readindex.storageRead.alignment": "1048576",
"durablelog.checkpoint.commit.count.min": "300",
"bookkeeper.ack.quorum.size": "3",
"controller.security.tls.enable": "true",
"controller.security.tls.server.certificate.location": "/etc/secret-volume/controller01.pem",
"controller.security.tls.server.privateKey.location": "/etc/secret-volume/controller01.key.pem",
"controller.security.tls.trustStore.location": "/etc/secret-volume/ca-cert",
"controller.security.tls.server.keyStore.location": "/etc/secret-volume/controller01.jks",
"controller.security.tls.server.keyStore.pwd.location": "/etc/secret-volume/pass-secret-tls",
"controller.security.pwdAuthHandler.accountsDb.location": "/etc/auth-passwd-volume/pass-secret-tls-auth.txt",
"pravegaservice.security.tls.enable": "true",
"pravegaservice.security.tls.server.certificate.location": "/etc/secret-volume/segmentstore01.pem",
"pravegaservice.security.tls.server.privateKey.location": "/etc/secret-volume/segmentstore01.key.pem",
"autoScale.controller.connect.security.tls.enable": "true",
"autoScale.controller.connect.security.tls.truststore.location": "/etc/secret-volume/ca-cert",
"bookkeeper.connect.security.tls.enable": "true",
"bookkeeper.connect.security.tls.trustStore.location": "empty",
"autoScale.controller.connect.security.tls.validateHostName.enable": "true",
"autoScale.controller.connect.security.auth.enable": "true",
"controller.security.auth.delegationToken.signingKey.basis": "secret",
"autoScale.security.auth.token.signingKey.basis": "secret",
"pravega.client.auth.token": "YWRtaW46MTExMV9hYWFh",
"pravega.client.auth.method": "Basic",
}
p.Spec.Pravega.SegmentStoreJVMOptions = []string{"-Xmx2g", "-XX:MaxDirectMemorySize=2g"}
p.Spec.Pravega.ControllerJvmOptions = []string{"-XX:MaxDirectMemorySize=1g"}

err := f.Client.Create(goctx.TODO(), p, &framework.CleanupOptions{TestContext: ctx, Timeout: CleanupTimeout, RetryInterval: CleanupRetryInterval})
Expand Down
35 changes: 32 additions & 3 deletions pkg/test/e2e/e2eutil/spec_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// NewDefaultCluster returns a cluster with an empty spec, which will be filled
// with default values
// NewDefaultCluster returns a cluster with some of the fields already filled in spec and the remaining fields
// will be filled with default values
func NewDefaultCluster(namespace string) *api.PravegaCluster {
return &api.PravegaCluster{
TypeMeta: metav1.TypeMeta{
Expand All @@ -33,14 +33,43 @@ func NewDefaultCluster(namespace string) *api.PravegaCluster {
Name: "pravega",
Namespace: namespace,
},
Spec: api.ClusterSpec{},
Spec: api.ClusterSpec{
Pravega: &api.PravegaSpec{
Options: map[string]string{"pravegaservice.cache.size.max": "1610612736"},
SegmentStoreJVMOptions: []string{"-Xmx1g", "-XX:MaxDirectMemorySize=2560m"},
SegmentStoreResources: &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1000m"),
corev1.ResourceMemory: resource.MustParse("4Gi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("2000m"),
corev1.ResourceMemory: resource.MustParse("4Gi"),
},
},
},
},
}
}

func NewClusterWithVersion(namespace, version string) *api.PravegaCluster {
cluster := NewDefaultCluster(namespace)
cluster.Spec = api.ClusterSpec{
Version: version,
Pravega: &api.PravegaSpec{
Options: map[string]string{"pravegaservice.cache.size.max": "1610612736"},
SegmentStoreJVMOptions: []string{"-Xmx1g", "-XX:MaxDirectMemorySize=2560m"},
SegmentStoreResources: &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1000m"),
corev1.ResourceMemory: resource.MustParse("4Gi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("2000m"),
corev1.ResourceMemory: resource.MustParse("4Gi"),
},
},
},
}
return cluster
}
Expand Down
36 changes: 19 additions & 17 deletions test/e2e/cm_spec_changes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ func testCMUpgradeCluster(t *testing.T) {
cluster := pravega_e2eutil.NewDefaultCluster(namespace)

cluster.WithDefaults()
jvmOpts := []string{"-XX:MaxDirectMemorySize=1g", "-XX:MaxRAMPercentage=50.0"}
cluster.Spec.Pravega.Options["pravegaservice.containerCount"] = "3"
cluster.Spec.Pravega.ControllerJvmOptions = jvmOpts
cluster.Spec.Pravega.SegmentStoreJVMOptions = jvmOpts
jvmOptsController := []string{"-XX:MaxDirectMemorySize=1g", "-XX:MaxRAMPercentage=50.0"}
jvmOptsSegmentStore := append(cluster.Spec.Pravega.SegmentStoreJVMOptions, "-XX:MaxRAMPercentage=50.0")
cluster.Spec.Pravega.Options["pravegaservice.container.count"] = "3"
cluster.Spec.Pravega.ControllerJvmOptions = jvmOptsController
cluster.Spec.Pravega.SegmentStoreJVMOptions = jvmOptsSegmentStore

pravega, err := pravega_e2eutil.CreatePravegaCluster(t, f, ctx, cluster)
g.Expect(err).NotTo(HaveOccurred())
Expand All @@ -62,17 +63,18 @@ func testCMUpgradeCluster(t *testing.T) {
// Check configmap has correct values
c_cm := pravega.ConfigMapNameForController()
ss_cm := pravega.ConfigMapNameForSegmentstore()
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, c_cm, "JAVA_OPTS", jvmOpts)
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, c_cm, "JAVA_OPTS", jvmOptsController)
g.Expect(err).NotTo(HaveOccurred())
jvmOpts = append(jvmOpts, "pravegaservice.service.listener.port=12345")
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, ss_cm, "JAVA_OPTS", jvmOpts)
jvmOptsSegmentStore = append(jvmOptsSegmentStore, "pravegaservice.service.listener.port=12345")
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, ss_cm, "JAVA_OPTS", jvmOptsSegmentStore)
g.Expect(err).NotTo(HaveOccurred())

//updating pravega options
jvmOpts = []string{"-XX:MaxDirectMemorySize=4g", "-XX:MaxRAMPercentage=60.0", "-XX:+UseContainerSupport"}
pravega.Spec.Pravega.ControllerJvmOptions = jvmOpts
pravega.Spec.Pravega.SegmentStoreJVMOptions = jvmOpts
pravega.Spec.Pravega.Options["bookkeeper.bkAckQuorumSize"] = "2"
jvmOptsController = []string{"-XX:MaxDirectMemorySize=4g", "-XX:MaxRAMPercentage=60.0", "-XX:+UseContainerSupport"}
jvmOptsSegmentStore = []string{"-Xmx1g", "-XX:MaxDirectMemorySize=2560m", "-XX:MaxRAMPercentage=60.0", "-XX:+UseContainerSupport"}
pravega.Spec.Pravega.ControllerJvmOptions = jvmOptsController
pravega.Spec.Pravega.SegmentStoreJVMOptions = jvmOptsSegmentStore
pravega.Spec.Pravega.Options["bookkeeper.ack.quorum.size"] = "2"
pravega.Spec.Pravega.Options["pravegaservice.service.listener.port"] = "443"
pravega.Spec.Pravega.SegmentStoreServiceAccountName = "pravega-components"
pravega.Spec.Pravega.ControllerServiceAccountName = "pravega-components"
Expand Down Expand Up @@ -102,24 +104,24 @@ func testCMUpgradeCluster(t *testing.T) {
time.Sleep(60 * time.Second)

// Check configmap is Updated
jvmOpts = append(jvmOpts, "bookkeeper.bkAckQuorumSize=2")
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, c_cm, "JAVA_OPTS", jvmOpts)
jvmOptsController = append(jvmOptsController, "bookkeeper.ack.quorum.size=2")
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, c_cm, "JAVA_OPTS", jvmOptsController)
g.Expect(err).NotTo(HaveOccurred())
jvmOpts = append(jvmOpts, "pravegaservice.service.listener.port=443")
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, ss_cm, "JAVA_OPTS", jvmOpts)
jvmOptsSegmentStore = append(jvmOptsSegmentStore, "pravegaservice.service.listener.port=443")
err = pravega_e2eutil.CheckConfigMapUpdated(t, f, ctx, pravega, ss_cm, "JAVA_OPTS", jvmOptsSegmentStore)
g.Expect(err).NotTo(HaveOccurred())

err = pravega_e2eutil.WriteAndReadData(t, f, ctx, pravega)
g.Expect(err).NotTo(HaveOccurred())

//updating pravega option
pravega.Spec.Pravega.Options["pravegaservice.containerCount"] = "10"
pravega.Spec.Pravega.Options["pravegaservice.container.count"] = "10"

//updating pravegacluster
err = pravega_e2eutil.UpdatePravegaCluster(t, f, ctx, pravega)

//should give an error
g.Expect(strings.ContainsAny(err.Error(), "controller.containerCount should not be changed")).To(Equal(true))
g.Expect(strings.ContainsAny(err.Error(), "controller.container.count should not be changed")).To(Equal(true))

// Delete cluster
err = pravega_e2eutil.DeletePravegaCluster(t, f, ctx, pravega)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/resources/kubernetes_master_install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ sudo chown $(id -u):$(id -g) $HOME/.kube/config
kubectl get nodes
kubectl apply -f https://docs.projectcalico.org/v3.14/manifests/calico.yaml
kubectl get nodes
sudo apt-get install binutils bison gcc -y
sudo apt-get install binutils bison gcc make -y
mkdir /export
2 changes: 1 addition & 1 deletion test/e2e/resources/kubernetes_slave_install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ sudo apt-get install docker-ce=5:19.03.9~3-0~ubuntu-focal -y
echo "docker install"
sudo systemctl enable --now docker
apt-get update && apt-get install -y \
apt-transport-https ca-certificates curl software-properties-common gnupg2
apt-transport-https ca-certificates curl software-properties-common gnupg2 make
echo "installed certs"
sudo curl -s https://packages.cloud.google.com/apt/doc/apt-key.gpg | sudo apt-key add -
echo "deb http://apt.kubernetes.io/ kubernetes-xenial main" \
Expand Down